Re: [Xen-devel] Libxenstore memory leak on static compile

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-09-12 at 16:09 +0100, Andres Lagar-Cavilla wrote:
> On Sep 12, 2012, at 11:03 AM, Roger Pau Monne wrote:
> 
> > Ian Campbell wrote:
> >> On Wed, 2012-09-12 at 14:44 +0100, Andres Lagar-Cavilla wrote:
> >>> When statically compiling libxenstore.a, the USE_PTHREAD define is not
> >>> applied. This results in cleanup_{pus/pop} being no-ops. cleanup_p*
> >>> are used throughout xs.c:read_message to free malloc'ed objects.
> >>> 
> >>> In short, libxenstore.a will leak memory when reading xenstore
> >>> messages. OOM killer awaits.
> >>> 
> >>> This could be solved by either turning on USE_PTHREAD for .a
> >>> compilation (which, N.B. will not actually link libpthread but instead
> >>> produce an object archive that needs to be eventually linked to
> >>> libpthread.so), or by replacing cleanup_p* by proper free calls.
> >> 
> >> The reason for the non-pthreads static library was so that you could
> >> build tiny statically linked xenstore clients against tiny libcs (like
> >> uclibc) to have things small enough to fit in e.g. your installer initrd
> >> or in your "guest tools" package.
> >> 
> >> It used to be that uclibc didn't have a pthreads library.  Maybe this
> >> has changed though (Roger, CCd, would know).
> > 
> > Yes, uclibc added a pthread library back in 2002:
> > 
> > http://mailman.uclinux.org/pipermail/uclinux-dev/2002-March/007613.html
> I have a simple fix to compile with pthread.h (below). It's really your call 
> as to whether you want to maintain pthread-free versions of libraries for the 
> next embedded femto-libc (and whether that is consistently applied throughout 
> the tree, etc). Getting rid of pthread_push/pop as alternative is not bad at 
> all.
> 
> Andres
> 
> # HG changeset patch
> # Parent b3a8ef4c556cc9a2d310cd8dd1e2f625c92c8515
> Compile xs.o with pthread support.
> 
> Otherwise messages are not freed in the statically linked libxenstore.a.

If you do this then you may as well nuke the !USE_PTHREAD case
altogether, since this makes it pointless.

I'd prefer not to do that though.

> 
> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> 
> diff -r b3a8ef4c556c tools/xenstore/Makefile
> --- a/tools/xenstore/Makefile
> +++ b/tools/xenstore/Makefile
> @@ -81,6 +81,7 @@ libxenstore.so.$(MAJOR): libxenstore.so.
>       ln -sf $< $@
>  
>  xs.opic: CFLAGS += -DUSE_PTHREAD
> +xs.o: CFLAGS += -DUSE_PTHREAD
>  
>  libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic
>       $(CC) $(LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenstore.so.$(MAJOR) 
> $(SHLIB_LDFLAGS) -o $@ $^ $(SOCKET_LIBS) -lpthread $(APPEND_LDFLAGS)
> 
> > 
> >> We don't seem to use pthread_cleanup_push/pop very extensively, so I
> >> think using proper free calls is probably the way to go?
> >> 
> >> Using that pthread facility as a cheap and nasty GC seems a bit wrong to
> >> me anyhow.
> >> 
> >> Ian.
> >> 
> > 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel