ANNOUNCEMENT: Live Wireshark University & Allegro Packets online APAC Wireshark Training Session
April 17th, 2024 | 14:30-16:00 SGT (UTC+8) | Online

Wireshark-dev: Re: [Wireshark-dev] ep_ memory is not garbage collected

From: Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>
Date: Mon, 20 Aug 2012 15:07:49 +0200
On Sun, Aug 19, 2012 at 08:29:37AM -0400, Evan Huus wrote:
> On Fri, Aug 17, 2012 at 10:02 AM, Jakub Zawadzki
> <darkjames-ws@xxxxxxxxxxxx> wrote:
> > On Thu, Aug 16, 2012 at 08:54:54PM -0400, Evan Huus wrote:
> >> On Wed, Aug 15, 2012 at 11:36 AM, Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx> wrote:
> >> > 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.
> >>
> >> Based on the log for r43188 I expect there's someway to force clear
> >> the GtkTreeStore, so that old_edt can be unrefed (triggering a gc)
> >> before cf->edt is re-allocated?
> >>
> >> Is your fix checked in or attached to a bug somewhere?
> >
> > Attached, but it's more workaround than fix.
> 
> Looks sane enough as far as workarounds go. I don't know if there's
> already a process for this, but I would commit that to trunk (with a
> comment in the code pointing to this discussion). 

Well, I'd rather not commit it, cause it only fix part of the problem.

> Once it's had a week or two of soak time, backport it to 1.8 and remove it from trunk.

This is not required, r43188 is not part of 1.8

> Any objections from the general list?
> 
> I'll try and take a look at a proper fix for trunk at some point, but

What do you think about reverting r42254 and using g_malloc() for data_source, 
like suggested by Anders? (https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5284#c26)

> I've been very busy with real life lately, so no promises. I'm pretty
> sure the stack idea from my previous email works if somebody else
> wants to grab this.

Relax, no one complained about this for 3 months, and I can wait for fix ;-)