Wireshark-dev: Re: [Wireshark-dev] ep_ memory is not garbage collected
From: Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>
Date: Wed, 15 Aug 2012 17:36:00 +0200
On Wed, Aug 15, 2012 at 10:22:13AM -0400, Evan Huus wrote:
> On Wed, Aug 15, 2012 at 9:53 AM, Jakub Zawadzki
> <darkjames-ws@xxxxxxxxxxxx> wrote:
> > From commit r42254 (reference counting of edt)
> > we have problem with ep_ memory not being returned to pool after dissecting packet.
> > Wireshark after initial opening file, always keep edt for currently selected packet,
> > so edt_refs is always > 0.
> >
> > You can reproduce it by loading some bigger files, and several times open expert
> > or conversation window.
> >
> > After r43188 (not part of wireshark-1.8) affected is also filtering (if at least one packet
> > match filter), or selecting other packets.
> 
> I'm not sure I follow. Are we calling epan_dissect_run() for each
> packet selected without calling
> epan_dissect_cleanup()/epan_dissect_init() in between? 

>From r43188 sequence of selecting new packet (cf_select_packet() in file.c) is:
   cf_read_frame()
   old_edt = cf->edt;
   cf->edt = epan_dissect_new()                ; edt_refs++
   epan_dissect_run(cf->edt, ...)
   if (old_edt)
     epan_dissect_free(old_edt);               ; edt_refs--  (edt_refs != 0, no gc)

Old code was doing something like:
   cf_read_frame()
   epan_dissect_free(cf->edt);               ; edt_refs--    (likely edt_refs == 0, gc ep memory)
   cf->edt = epan_dissect_new()              ; edt_refs++

I have working fix for this, but check below.

> I understand that to be a bug in the caller, not a bug in ep_'s memory manager, or
> did r42254 change the behaviour of the API in an unexpected way?

cf_select_packet() don't call epan_dissect_free() but holds edt reference in cf->edt,
so if we have selected packet than edt_refs > 0 (this is also true before r43188).

If in such situation we do something which requires redissecting lot of packet (or do it several times)
wireshark will eat all your memory ;-)

I think it'd be best if each dissection have unique, single ep_pool (no ref-counting),
but it's lot of work, so we need some workaround, also for 1.8 ;/