Wireshark-dev: Re: [Wireshark-dev] [Bug 7775] Wireshark leaks memory when selecting packets
From: Evan Huus <eapache@xxxxxxxxx>
Date: Mon, 8 Oct 2012 18:29:33 -0400
On Mon, Oct 8, 2012 at 5:17 PM, Jakub Zawadzki
<darkjames-ws@xxxxxxxxxxxx> wrote:
> On Mon, Oct 08, 2012 at 04:29:15PM -0400, Evan Huus wrote:
>> On Mon, Oct 8, 2012 at 3:31 PM, Jakub Zawadzki
>> <darkjames-ws@xxxxxxxxxxxx> wrote:
>> > Hi Evan,
>> >
>> > On Mon, Oct 08, 2012 at 03:06:29PM -0400, Evan Huus wrote:
>> >> On Mon, Oct 8, 2012 at 2:58 PM,  <bugzilla-daemon@xxxxxxxxxxxxx> wrote:
>> >> > https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7775
>> >> >
>> >> > --- Comment #10 from Anatoly <aries.nah@xxxxxxxxx> 2012-10-08 14:58:26 EDT ---
>> >> > I've tried r45395, nothin is changed except wireshark has crashed:)
>> >> > But I got one more probably interesting fact: r45380 (w/o your fixes) on Vista
>> >> > 32 bit - memory consumption increases slightly and sometimes it decreases, but
>> >> > the same revision of XP 32bit - it always increases by ~2MB.
>> >>
>> >> This appears to be an XP-specific bug if anyone still has an XP
>> >> install to test with (I don't).
>> >
>> > It mighe be OOM, for every packet we mmap 10MiB chunk of memory.
>> > Which is *not* unmaped in emem_free_all() nor ep_free_pool().
>> > (AFAIR for performance reasons)
>> >
>> > Generally it'd be best to save somewhere mem->free_list before
>> > 1333         g_free(mem);
>> >
>> > If it's hard to do than emem_destroy_chunk() is your friend.
>> >
>> > [PS: I haven't test it yet, but looking at diff it makes sense,
>> >      if not please move on ;)]
>>
>> I think you're right - I did all of my testing of the emem changes
>> under the valgrind-wireshark.sh script, which disables chunks (and
>> thus the mmap) so I never noticed it before. I have a patch (attached)
>> which I believe ought to correctly share the free_list between the
>> various pools, but for some reason I can't seem to track down it's
>> causing random canary errors.
>>
>> Any ideas?
>
> It works with your patch + attached patch.

It doesn't crash, yes, but it leaks again. I've added
emem_destroy_chunk() for now in revision 45412, until I can spend some
time figuring out the correct logic for sharing freed blocks.