Bug 42942 - memory leak in pa_threaded_mainloop_free
Summary: memory leak in pa_threaded_mainloop_free
Status: RESOLVED NOTABUG
Alias: None
Product: PulseAudio
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: pulseaudio-bugs
QA Contact: pulseaudio-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-15 01:55 UTC by Peter Meerwald
Modified: 2012-03-15 06:57 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
core: Detect valgrind for real (2.15 KB, patch)
2011-12-30 10:37 UTC, Arun Raghavan
Details | Splinter Review

Description Peter Meerwald 2011-11-15 01:55:18 UTC
the following program leaks memory according to valgrind; latest git source build with CFLAGS="-ggdb3 -O0" LDFLAGS="-ggdb3" ./configure --enable-static 

#include "pulse/pulseaudio.h"
int main() {
    pa_threaded_mainloop *ml = pa_threaded_mainloop_new();
    pa_threaded_mainloop_free(ml);
}

==18066== HEAP SUMMARY:
==18066==     in use at exit: 44 bytes in 2 blocks
==18066==   total heap usage: 12 allocs, 10 frees, 621 bytes allocated
==18066== 
==18066== 4 bytes in 1 blocks are still reachable in loss record 1 of 2
==18066==    at 0x4C28FAC: malloc (vg_replace_malloc.c:236)
==18066==    by 0x437C0B: pa_xmalloc (xmalloc.c:65)
==18066==    by 0x466625: _pa_xnew_internal (xmalloc.h:66)
==18066==    by 0x466FE3: pa_tls_new (thread-posix.c:220)
==18066==    by 0x46679D: current_thread_tls_init (thread-posix.c:66)
==18066==    by 0x453E50: pa_run_once (once.c:91)
==18066==    by 0x4667B9: current_thread_tls_obj (thread-posix.c:66)
==18066==    by 0x466832: current_thread_tls_get (thread-posix.c:66)
==18066==    by 0x466C9F: pa_thread_self (thread-posix.c:144)
==18066==    by 0x4309DC: in_worker (thread-mainloop.c:56)
==18066==    by 0x430C8A: pa_threaded_mainloop_free (thread-mainloop.c:122)
==18066==    by 0x406083: main (pa-test2.cpp:266)
==18066== 
==18066== 40 bytes in 1 blocks are still reachable in loss record 2 of 2
==18066==    at 0x4C279FC: calloc (vg_replace_malloc.c:467)
==18066==    by 0x437CF1: pa_xmalloc0 (xmalloc.c:76)
==18066==    by 0x466675: _pa_xnew0_internal (xmalloc.h:77)
==18066==    by 0x466CBF: pa_thread_self (thread-posix.c:150)
==18066==    by 0x4309DC: in_worker (thread-mainloop.c:56)
==18066==    by 0x430C8A: pa_threaded_mainloop_free (thread-mainloop.c:122)
==18066==    by 0x406083: main (pa-test2.cpp:266)
Comment 1 Peter Meerwald 2011-11-15 02:47:44 UTC
rerunning with
VALGRIND=1 valgrind --leak-check=full --show-reachable=yes ./pa-test2
and the leak went away...

seriously, this is garbage and undocumented (?) until you end up looking at
#define PA_STATIC_TLS_DECLARE(name, free_cb) 
to find out that the destructor 

    static void name##_tls_destructor(void) {                           \
        static void (*_free_cb)(void*) = free_cb;                       \
        if (!pa_in_valgrind())                                          \
            return;                                                     \

just returns if the environment variable (!!!) VALGRIND is not set

why to check for valgrind/memcheck.h in configure.ac then?

I strongly suggest to drop the pa_in_valgrind() checks in various destructors (thread/flist) altogether 

if I call something_free() I expect that something is actually freed, and NOT that someone is supersmart about it (for what reason anyway?)

annoyed...
Comment 2 Arun Raghavan 2011-12-30 10:37:42 UTC
Created attachment 54980 [details] [review]
core: Detect valgrind for real

I'm not committing this patch yet to see if anyone knows about what heisenbugs RUNNING_ON_VALGRIND might cause. This macro is used in GStreamer as well, and I'm not aware of any issues there.

If there are no objections, I'll go ahead and push this.
Comment 3 Arun Raghavan 2012-03-14 23:25:01 UTC
After chatting with Lennart about this, the current behaviour is not avoidable because of threading issues. Unfortunately, I've forgotten the specifics and lost IRC logs. Will poke one of the others to see if they can post the conversation here or on the wiki for posterity.
Comment 4 Tanu Kaskinen 2012-03-15 06:57:35 UTC
2011/12/31:01:41 < mezcalero> coling: hmm, Ford_Prefect was asking about RUNNING_ON_VALGRIND and not freeing static data in non-valgrind mode
2011/12/31:01:42 < mezcalero> coling: now he is offline
2011/12/31:01:42 < mezcalero> coling: just wanted to give the answer
2011/12/31:01:42 < mezcalero> coling: so that you guys know what this is about
2011/12/31:01:42 < mezcalero> coling: basically, the freeing of static data is done for gcc destructors
2011/12/31:01:42 < mezcalero> and gcc destructors are fucked up in multi-threaded programs and in the context of libraries
2011/12/31:01:42 < mezcalero> coling: which is why we normally don't do anything in them
2011/12/31:01:43 < mezcalero> coling: except if you set RUNNING_ON_VALGRIND
2011/12/31:01:43 < mezcalero> coling: the assumption here is that you set that env var only when in single threaded apps
2011/12/31:01:43 < mezcalero> coling: with other words: this logic matters, and can't simply be dropped
2011/12/31:01:43 < mezcalero> coling: and it is not about speeding things up, it is about ensuring that multi-threaded apps work correctly

Also, later:

2012/01/02:12:52 < Ford_Prefect> mezcalero: thanks for explaining. I'll fix things up and dump this as documentation somewhere for future generations.

...just as a reminder ;)


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.