Wireshark-bugs: [Wireshark-bugs] [Bug 5284] new_packet_list: redissection + redraw crashes when
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5284
--- Comment #13 from Evan Huus <eapache@xxxxxxxxx> 2012-04-24 12:43:15 PDT ---
Created attachment 8312
--> https://bugs.wireshark.org/bugzilla/attachment.cgi?id=8312
Use per-edt ephemeral memory pools.
The attached patch has some known problems, but it does fix the bug in
question, and I hope that it provides at least a starting point for a real fix.
This is the first time I've delved into emem, so I apologize if I've done
things that aren't kosher with the intended design. Constructive criticisms are
very welcome.
(Fair warning: The patch changes epan/emem.h slightly, so applying it will
require re-compiling almost the entire tree. Sorry.)
----------------
Explanation
----------------
- I replaced the single ephemeral memory pool with a GHashTable mapping void
pointers to memory pools, and created a single static-global key.
- ep_alloc() gets its pool from the hashtable (using the global key). If it
doesn't find a pool, it creates one and adds it to the hashtable.
- ep_free_all() takes a void pointer key. It updates the global key, and cleans
up the memory pool at that key (if there is one).
- ep_dissect_run() passes the address of its epan_dissect_t as the key to
ep_free_all().
- ep_dissect_cleanup now calls ep_free_all() (it didn't before) with the
address of the epan_dissect_t.
-----------------
Possible Issues
-----------------
- The functions ep_check_canary_integrity(), print_alloc_stats() and
ep_verify_pointer() all have minor changes in the patch to make them compile,
but are probably no longer correct. I assume that the right changes for these
functions would be to make them iterate over all the pools in the hashtable?
- Performance is no longer optimal. I've done no formal performance testing,
but I can't imagine adding a hashtable lookup to every ep_alloc() is doing it
any good. Additionally, there's more heap work since every ephemeral pool
header is now on the heap rather than the stack.
- I'm not sure if the ep_free_all() call in ep_dissect_cleanup() is necessarily
correct. I added it to prevent leaking emem_header_t's, and it hasn't caused
any problems in any of the testing I've done, but I'm not 100% clear on when
it's expected to be called.
-----------------
Conclusion
-----------------
If the general direction is approved of, I will clean up any problems and post
a nicer patch for check-in. If someone has a different direction in mind, I'm
willing to try and code it up.
Thanks,
Evan
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
