GNOME Bugzilla – Bug 793026
possible mem leak in g_mutex_impl_new
Last modified: 2018-02-06 15:52:59 UTC
In g_mutex_impl_new we have #ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP { pthread_mutexattr_t attr; pthread_mutexattr_init (&attr); pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_ADAPTIVE_NP); pattr = &attr; } #endif attr context is within PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP and braces. Later on, to destroy attr we have #ifdef PTHREAD_ADAPTIVE_MUTEX_NP pthread_mutexattr_destroy (&attr); #endif which is within PTHREAD_ADAPTIVE_MUTEX_NP so not PTHREAD_ADAPTIVE_MUTEX_INITIALIZED_NP but even if we use the correct ifdef, having the declaration of attr inside braces, it is not in the same scope
Created attachment 367738 [details] [review] gthread: Fix a typo in an #ifdef on the non-native mutex path This seems to have been present since the code was introduced in commit cedc82290f860683d695d0c5326db153893eec21. The attr variable is defined under one #ifdef, but destroyed under another, which doesn’t make any sense. The second #ifdef variable is actually an enum value, rather than the static initialiser value which makes more sense in the context. Note that GMutex used to be statically initialised to the value of PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP in gthread.h, before this was reworked in commit e081eadda598bc708fbf9dd53a190fc3b0e7fa76. Signed-off-by: Philip Withnall <withnall@endlessm.com>
This does indeed look like a bug, thanks, although it’s not one which will typically be hit, because it’s in the non-native mutex implementation path. Typically, GLib on Linux will use native futexes.
Just for clarification: attr declaration is surrounded by braces. Doesn't it mean that it's scope is visible only there and not inside the second ifdef where we destroy it? I thought we should remove the braces in the declaration or do something like this #ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP { pthread_mutexattr_t attr; pthread_mutexattr_init (&attr); pthread_mutexattr_settype (&attr, PTHREAD_MUTEX_ADAPTIVE_NP); pattr = &attr; #endif ... #ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP pthread_mutexattr_destroy (&attr); } #endif
(In reply to Daniele from comment #3) > Just for clarification: attr declaration is surrounded by braces. Doesn't it > mean that it's scope is visible only there and not inside the second ifdef > where we destroy it? What version of GLib are you looking at? The braces were removed in commit 03a82ce898ea435efd73a250d989f8dc6249aaf8, in 2014. git master currently looks like this: https://git.gnome.org/browse/glib/tree/glib/gthread-posix.c
Sorry. Just realized my link was on github and not on git.gnome. https://github.com/djdeath/glib/blob/android/glib/gthread-posix.c Suppose this is the android branch.
I have no idea what that branch is meant to do (it’s not official). It’s pretty old. I believe Android support works on master when building with Meson, although I haven’t tested it. If you are trying to use GLib on Android, and have problems, please file more bugs or get in touch with people on IRC. In any case, there is still a bug here on git master, which my patch should fix. It’s waiting for another developer to review it.
Review of attachment 367738 [details] [review]: sure
Pushed to master, thanks Matthias! Attachment 367738 [details] pushed as 79d9ea2 - gthread: Fix a typo in an #ifdef on the non-native mutex path