Wireshark-dev: Re: [Wireshark-dev] RFD: The Future of Memory Management in Wireshark

From: Evan Huus <eapache@xxxxxxxxx>
Date: Sun, 28 Oct 2012 19:45:10 -0400
On Sun, Oct 28, 2012 at 7:26 PM, Jakub Zawadzki
<darkjames-ws@xxxxxxxxxxxx> wrote:
> On Sun, Oct 28, 2012 at 10:12:41AM -0400, Evan Huus wrote:
>> We might be able to fake the proper scoping using thread-local globals
>> if we wrap everything in functions that assert the state of a
>> dissection. Something like:
>>
>> __thread wmem_allocator_t *packet_scope;
>> __thread gboolean packet_in_scope = FALSE;
>
> Just a note before commiting:
>
> __thread is GCC extension, MVSC has got __declspec(thread) and
> C11 has got _Thread_local.
>
> So to be portable we must use glib GStaticPrivate.

Yes, I just wanted to be consistent with Dirk's previous example.

>> wmem_allocator_t *
>> wmem_packet_scope(void)
>> {
>>   g_assert(packet_in_scope);
>>   return packet_scope;
>> }
>>
>> void
>> wmem_start_packet_scope(void)
>> {
>>   g_assert(!packet_in_scope);
>>   packet_in_scope = TRUE;
>> }
>>
>> void
>> wmem_stop_packet_scope(void)
>> {
>>   g_assert(packet_in_scope);
>>   packet_in_scope = FALSE;
>>   wmem_free_all(packet_scope);
>> }
>>
>> Invalid accesses would still compile, but at least they would throw an
>> assertion as soon as the code path was hit. Thoughts on something like
>> this?
>
> I'm not sure if you want to replace all ep_ pool with packet_scope pool,
> but if yes what kind of memory you want to use in GUI, or for error messages
> in epan/dfilter/? glib allocator?

I think the current scope for ep_ memory is basically correct now,
thanks to your work moving ep_free_all to after dissect_packet. The
epan_dissect_run and epan-dissect_run_with_taps functions would
basically be:

wmem_start_packet_scope();
dissect_packet();
wmem_stop_packet_scope();

ep_ memory that is currently being allocated outside of that scope
would have to be converted on a case by case basis - I suspect some of
it would be trivial to safely convert to glib. As Jeff pointed out in
your thread, preventing leaks when exceptions are thrown is the
biggest win for emem and wmem, so if it's being used in a scope where
exceptions aren't thrown then it should be sane (if not easy) to
convert to glib. Other cases (liked dfilter) might warrant having its
own internal scope, which wmem makes it possible to do sanely.

Tangentially, I haven't noticed if you've checked in a pinfo memory
scope yet or not. Is there anything else wmem needs to be able to do
for that to work or did you end up going in a different direction?

Cheers,
Evan