On Mon, Oct 08, 2012 at 04:29:15PM -0400, Evan Huus wrote:
> On Mon, Oct 8, 2012 at 3:31 PM, Jakub Zawadzki
> <[email protected]> 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,  <[email protected]> wrote:
> >> > https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7775
> >> >
> >> > --- Comment #10 from Anatoly <[email protected]> 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.
diff --git a/epan/emem.c b/epan/emem.c
index 12a3070..b506bcf 100644
--- a/epan/emem.c
+++ b/epan/emem.c
@@ -1325,9 +1325,11 @@ ep_create_pool(void)
        /* The free_list, specifically when using chunks, does
         * not get freed (or unmapped, as the case may be) by
         * emem_free_all(), so we share it between the pools. */
-       if (ep_packet_mem->debug_use_chunks) {
+       /* We can only share unused chunks, so check if 
->free_list->canary_last is NULL */
+       if (ep_packet_mem->debug_use_chunks && ep_packet_mem->free_list && 
ep_packet_mem->free_list->canary_last == NULL) {
                mem->free_list = ep_packet_mem->free_list;
                ep_packet_mem->free_list = NULL;
+               g_print("stolen\n");
        }
 
        ep_pool_stack = g_slist_prepend(ep_pool_stack, mem);
@@ -1362,8 +1364,14 @@ ep_free_pool(emem_pool_t *mem)
         * via ep_create_pool() and so copying NULL over it here would
         * just leak memory. */
        if (ep_packet_mem->debug_use_chunks && free_list != NULL) {
-               g_assert(ep_packet_mem->free_list == NULL);
-               ep_packet_mem->free_list = free_list;
+               emem_chunk_t *p = ep_packet_mem->free_list;
+
+               if (p) {
+                       while (p->next)
+                               p = p->next;
+                       p->next = free_list;
+               } else
+                       ep_packet_mem->free_list = free_list;
        }
 }
 
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <[email protected]>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

Reply via email to