[PATCH] compsys maps anonymous memory and never frees it

[PATCH] compsys maps anonymous memory and never frees it

["xRaich[o]²x"]

Bart Schaefer wrote:
> On Sep 4,  1:04am, =?ISO-8859-1?Q?Bj=F6rn_Herzig?= wrote:
> }
> } I looked at the problem a little closer. Zsh does not call mmap to
> } allocate them and they dont get allocated when completion happens but
> } when the next command gets issued.
>
> If this is true, then this is something happening down in the library
> or kernel implementation of fork() and is out of zsh's control.
>
> Did you build zsh yourself?  Can you check config.h for USE_MMAP ?
> If USE_MMAP is defined then anytime zsh parses a command it will have
> called mmap() to allocate zsh-heap space.  You can try reconfiguring
> with --enable-zsh-mem and then check the pmap behavior again.
>
> } So in my example the new maps got added to the process' address space
> } when i executed pmap, but the same happens with any other programm.
> } Builtins however are an exception. So things start to go wrong when it
> } comes to forking.
>
> If you run pmap from another shell window rather than executing it
> from within the shell whose map you're examining, does the behavior
> change at all?
>
> My only guess goes something like this:
>
> Zsh has mapped memory for the heap during parsing etc.  Those pages
> have had data written and therefore are marked "dirty".  When fork()
> is called, those pages become shared address space with the child
> process.  Zsh munmap()s them later but they aren't returned to the
> system because the child process is still using them.
>
> I'm not really happy with any of these explanations yet.
>
>   

Ok, i found it.... after wandering around in a few deadends and getting 
some stuff wrong... but well.

i did the following change to mem.c

mod_export void
old_heaps(Heap old)
{
    Heap h, n;

    queue_signals();
    for (h = heaps; h; h = n) {
    n = h->next;
    DPUTS(h->sp, "BUG: old_heaps() with pushed heaps");
#ifdef USE_MMAP
    //munmap((void *) h, sizeof(*h));
    munmap((void *) h, h->size);
#else
    //zfree(h, sizeof(*h));
    zfree(h, h->size);
#endif
    }
    heaps = old;
    fheap = NULL;
    unqueue_signals();
}

this might open up a can of worms. i don't know where this is going to 
blow up, but i'm pretty sure it will. but well... it seems to work.
old_heaps tried to unmap a 16384 bytes big segment by calling 
munmap(...,sizeof(h*)) which is 16 and left the rest in memory (which 
explains why the segments wasn't added explicitly).

Here is the dtrace script that finally caught the bug, if someone is 
interested:

#!/usr/sbin/dtrace -s

BEGIN
{
    printf("\nTarget is : %d\nSegement is %X\n",$target, $1);
}

syscall::mmap64:return
/arg1 == $1 && pid == $target/
{
    printf("mmap returns: %x",arg1);
    self->triggered = 1;
}

pid$target:zsh:dupstring:return
/self->triggered == 1/
{
    printf("Dupstring: %s",copyinstr(arg1));
    self->triggered = 0;
}

syscall::mmap64:entry
/pid == $target/
{
    printf("mmap size: %d",arg1);
}

syscall::munmap:entry
/arg0 == $1 && pid == $target/
{
    printf("munmap size: %d\n",arg1);
    ustack();
    self->munmapping = 1;
}

syscall::munmap:return
/self->munmapping/
{
    printf("Result of munmapping the segment in question was: %d",arg1);
    self->munmapping = 0;
}

fbt::as_removeseg:entry
/execname == "zsh" && ((struct seg*)arg1)->s_base == (caddr_t)$1/
{
    printf("\nGot removed");
    ustack(50);
    stack();
    exit(0);
}

fbt::as_addseg:entry
/execname == "zsh" && ((struct seg*)arg1)->s_base == (caddr_t)$1/
{
    printf("\nGot added");
    ustack();
    stack();
}

Regards,
Björn

Re: [PATCH] compsys maps anonymous memory and never frees it

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 3168 bytes --]

On Sunday 07 September 2008, xRaich[o]²x wrote:
> 
> Bart Schaefer wrote:
> > On Sep 4,  1:04am, =?ISO-8859-1?Q?Bj=F6rn_Herzig?= wrote:
> > }
> > } I looked at the problem a little closer. Zsh does not call mmap to
> > } allocate them and they dont get allocated when completion happens but
> > } when the next command gets issued.
> >
> > If this is true, then this is something happening down in the library
> > or kernel implementation of fork() and is out of zsh's control.
> >
> > Did you build zsh yourself?  Can you check config.h for USE_MMAP ?
> > If USE_MMAP is defined then anytime zsh parses a command it will have
> > called mmap() to allocate zsh-heap space.  You can try reconfiguring
> > with --enable-zsh-mem and then check the pmap behavior again.
> >
> > } So in my example the new maps got added to the process' address space
> > } when i executed pmap, but the same happens with any other programm.
> > } Builtins however are an exception. So things start to go wrong when it
> > } comes to forking.
> >
> > If you run pmap from another shell window rather than executing it
> > from within the shell whose map you're examining, does the behavior
> > change at all?
> >
> > My only guess goes something like this:
> >
> > Zsh has mapped memory for the heap during parsing etc.  Those pages
> > have had data written and therefore are marked "dirty".  When fork()
> > is called, those pages become shared address space with the child
> > process.  Zsh munmap()s them later but they aren't returned to the
> > system because the child process is still using them.
> >
> > I'm not really happy with any of these explanations yet.
> >
> >   
> 
> Ok, i found it.... after wandering around in a few deadends and getting 
> some stuff wrong... but well.
> 
> i did the following change to mem.c
> 
> mod_export void
> old_heaps(Heap old)
> {
>     Heap h, n;
> 
>     queue_signals();
>     for (h = heaps; h; h = n) {
>     n = h->next;
>     DPUTS(h->sp, "BUG: old_heaps() with pushed heaps");
> #ifdef USE_MMAP
>     //munmap((void *) h, sizeof(*h));
>     munmap((void *) h, h->size);

Good catch (it was obvious bug)

> #else
>     //zfree(h, sizeof(*h));
>     zfree(h, h->size);

In most other places we are using zfree(h, HEAPSIZE); there is no guarantee
heap is actually of size HEAPSIZE, but that does not matter (second argument
is used only with enable-zsh-mem and serves as plausibility check only).

Here is patch in proper format. It is using HEAPSIZE for consistency:

Index: Src/mem.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/mem.c,v
retrieving revision 1.16
diff -u -p -r1.16 mem.c
--- Src/mem.c   17 May 2008 17:55:38 -0000      1.16
+++ Src/mem.c   7 Sep 2008 09:23:32 -0000
@@ -153,9 +153,9 @@ old_heaps(Heap old)
        n = h->next;
        DPUTS(h->sp, "BUG: old_heaps() with pushed heaps");
 #ifdef USE_MMAP
-       munmap((void *) h, sizeof(*h));
+       munmap((void *) h, h->size);
 #else
-       zfree(h, sizeof(*h));
+       zfree(h, HEAPSIZE);
 #endif
     }
     heaps = old;

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] compsys maps anonymous memory and never frees it

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

On Sunday 07 September 2008, xRaich[o]²x wrote:
> 
> Bart Schaefer wrote:
> > On Sep 4,  1:04am, =?ISO-8859-1?Q?Bj=F6rn_Herzig?= wrote:
> > }
> > } I looked at the problem a little closer. Zsh does not call mmap to
> > } allocate them and they dont get allocated when completion happens but
> > } when the next command gets issued.
> >

I was able to more or less reproduce it on Linux; but now after your patch
I get what looks like memory leak. Here are heaps after several memory
intensive completions:

Address   Kbytes     RSS    Anon  Locked Mode   Mapping
08048000     608       -       -       - r-x--  zsh
080e0000      16       -       -       - rw---  zsh
080e4000      92       -       -       - rw---    [ anon ]
09d6a000     672       -       -       - rw---    [ anon ]

[...]

Address   Kbytes     RSS    Anon  Locked Mode   Mapping
08048000     608       -       -       - r-x--  zsh
080e0000      16       -       -       - rw---  zsh
080e4000      92       -       -       - rw---    [ anon ]
09d6a000    5340       -       -       - rw---    [ anon ]
b6a68000   15464       -       -       - rw---    [ anon ]


Compare this with version without patch:

Address   Kbytes     RSS    Anon  Locked Mode   Mapping
08048000     528       -       -       - r-x--  zsh
080cc000      16       -       -       - rw---  zsh
080d0000      88       -       -       - rw---    [ anon ]
099df000     744       -       -       - rw---    [ anon ]

[...]

Address   Kbytes     RSS    Anon  Locked Mode   Mapping
08048000     528       -       -       - r-x--  zsh
080cc000      16       -       -       - rw---  zsh
080d0000      88       -       -       - rw---    [ anon ]
099df000    5520       -       -       - rw---    [ anon ]
b6adb000   15464       -       -       - rw---    [ anon ]
b7a05000      12       -       -       - rw---    [ anon ]
b7b46000     108       -       -       - rw---    [ anon ]

So it looks like we still have some memory leak somewhere. 

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] compsys maps anonymous memory and never frees it

["xRaich[o]²x"]

Andrey Borzenkov wrote:
> On Sunday 07 September 2008, xRaich[o]²x wrote:
>   
>> Bart Schaefer wrote:
>>     
>>> On Sep 4,  1:04am, =?ISO-8859-1?Q?Bj=F6rn_Herzig?= wrote:
>>> }
>>> } I looked at the problem a little closer. Zsh does not call mmap to
>>> } allocate them and they dont get allocated when completion happens but
>>> } when the next command gets issued.
>>>
>>>       
>
> I was able to more or less reproduce it on Linux; but now after your patch
> I get what looks like memory leak. Here are heaps after several memory
> intensive completions:
>
> Address   Kbytes     RSS    Anon  Locked Mode   Mapping
> 08048000     608       -       -       - r-x--  zsh
> 080e0000      16       -       -       - rw---  zsh
> 080e4000      92       -       -       - rw---    [ anon ]
> 09d6a000     672       -       -       - rw---    [ anon ]
>
> [...]
>
> Address   Kbytes     RSS    Anon  Locked Mode   Mapping
> 08048000     608       -       -       - r-x--  zsh
> 080e0000      16       -       -       - rw---  zsh
> 080e4000      92       -       -       - rw---    [ anon ]
> 09d6a000    5340       -       -       - rw---    [ anon ]
> b6a68000   15464       -       -       - rw---    [ anon ]
>
>
> Compare this with version without patch:
>
> Address   Kbytes     RSS    Anon  Locked Mode   Mapping
> 08048000     528       -       -       - r-x--  zsh
> 080cc000      16       -       -       - rw---  zsh
> 080d0000      88       -       -       - rw---    [ anon ]
> 099df000     744       -       -       - rw---    [ anon ]
>
> [...]
>
> Address   Kbytes     RSS    Anon  Locked Mode   Mapping
> 08048000     528       -       -       - r-x--  zsh
> 080cc000      16       -       -       - rw---  zsh
> 080d0000      88       -       -       - rw---    [ anon ]
> 099df000    5520       -       -       - rw---    [ anon ]
> b6adb000   15464       -       -       - rw---    [ anon ]
> b7a05000      12       -       -       - rw---    [ anon ]
> b7b46000     108       -       -       - rw---    [ anon ]
>
> So it looks like we still have some memory leak somewhere. 
>   
Are you sure that those segments aren't those dlopen allocates when 
loading modules? they are actually created by the operating system and 
needed.

What i find a bit disturbing is that whenever loading modules one 
segment gets unmapped by munmap with two times its size. I think the 
operating system will catch that but maybe this is possible to fix 
somehow. i wrote a little dtrace script that keeps track of every mapped 
segment and checks if they get munmapped with the correct size. 
Everything except the mentioned munmap seems to work after the patch.

Regards,
Björn