Summary: | memory leak in pa_threaded_mainloop_free | ||
---|---|---|---|
Product: | PulseAudio | Reporter: | Peter Meerwald <pmeerw> |
Component: | core | Assignee: | pulseaudio-bugs |
Status: | RESOLVED NOTABUG | QA Contact: | pulseaudio-bugs |
Severity: | normal | ||
Priority: | medium | CC: | lennart, pmeerw |
Version: | unspecified | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: | core: Detect valgrind for real |
Description
Peter Meerwald
2011-11-15 01:55:18 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... 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. 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. 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.