Wireshark-bugs: [Wireshark-bugs] [Bug 5284] new_packet_list: redissection + redraw crashes when

Date: Wed, 25 Apr 2012 07:06:58 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5284

--- Comment #16 from Evan Huus <eapache@xxxxxxxxx> 2012-04-25 07:06:57 PDT ---
(In reply to comment #14)
> Hi,
> I think it looks good, but could we use an fixed array instead of the hash
> table? Will there ever be more than a small number of edt's active(<5)?

I wasn't sure, so I played it safe. I don't *think* there will ever be more
than a few active at once, but I don't know enough to be comfortable making
that assumption.

> Use global index into the array and it should be as fast as the "old"
> implementation.

Sounds good, but how would we translate the key pointer into an array index?

Honestly, if the GHashTable is implemented sanely using multiplicative hashing,
then a hash lookup shouldn't be much more than 5 instructions (noop, mult, sub,
shift, fetch) with a few more in there for the function call itself. While it
is a performance hit, I'm not sure it's necessarily something to worry about
unless testing reveals an actual visible slowdown.

(In reply to comment #15)
> Hmm - would moving ep_free_all() from  epan_dissect_run() to
> epan_dissect_cleanup() solve the whole problem?

It appears to, but I have no idea why.

My original thought when looking at this problem was to move ep_free_all() to
immediately after (instead of immediately before) the dissect_packet() call in
epan_dissect_run() on the assumption that the memory could be freed once the
dissection was actually run. This causes a segfault on every packet, however,
because ephemeral memory is being used in GTK calls that aren't finished yet
when the dissection is finished.

This worries me, because I don't know if we have any guarantees from GTK when
those calls will actually be finished. Yes it seems right now that they're all
being triggered in a reasonably time, but it seems legal (if odd) to me for GTK
to delay those calls such that we can never validly free ephemeral memory
without possibly running into this same sort of issue.

I may be revealing my ignorance of Wireshark's and GTK's architectures, but the
way ephemeral memory needs to be valid even after dissect_packet() is done
gives me the willies.

-- 
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.