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

[Andres Lagar-Cavilla <andreslc@xxxxxxxxxxxxxx>]

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.

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