GNOME Bugzilla – Bug 139699
Correction for g_main_context_unref()
Last modified: 2014-01-19 23:03:11 UTC
File: glib-2.4.0/glib/gmain.c Function: g_main_context_unref() When sources are added to the pending list, they are ref(). (line 2408) But, when the pending list is destroyed, sources MUST be unref(). This has been forgotten in 'g_main_context_unref()'. It should look like : void g_main_context_unref (GMainContext * context) { .................................. G_LOCK (main_context_list); main_context_list = g_slist_remove (main_context_list, context); G_UNLOCK (main_context_list); /* Destroy pending sources list */ ==> for (i = 0; i < context->pending_dispatches->len; i++) { ==> if (context->pending_dispatches->pdata[i]) ==> SOURCE_UNREF ((GSource *) context->pending_dispatches->pdata[i], ==> context); ==> } g_ptr_array_free (context->pending_dispatches, TRUE); source = context->source_list; while (source) { GSource *next = source->next; g_source_destroy_internal (source, context, TRUE); source = next; } .................................. }
I'm not sure if this is needed. If you look up a bit you'll see that all sources from context->source_list are destroyed. It seems to me that the pending_dispatches are always from that list, so they'll be gone anyway at that point...
My point of view: 1) A source attached to a context is ref(). (line 929 + g_source_list_add() ) 2) That source, when added to the pending list, is also ref(). (line 2408 + g_ptr_array_add() ) Now, when calling g_main_context_unref(); you break type 1) links. Sources are marked as destroyed, and unref() by g_source_destroy_internal(): source = context->source_list; while (source) { GSource *next = source->next; g_source_destroy_internal (source, context, TRUE); source = next; } .................................. But, before freeing context and the array of pending_dispatches by g_ptr_array_free (context->pending_dispatches, TRUE); the type 2) links must be break by unreferencing the sources again. Missing is the same piece of code as in g_main_context_prepare(); or in g_main_dispatch() where sources are unref() before g_ptr_array_set_size (context->pending_dispatches, 0);
The analysis looks right to me. But I have a hard time coming up with a non-broken example where g_main_context_unref () could be called with pending_dispatches being nonempty. Here is an example which does in fact manage to loose reference to a source in the way described by Philippe, but it doesn't matter much since we segfault a millisecond later anyway. #include <glib.h> static gboolean foo1 (gpointer data) { GMainContext *context = (GMainContext *)data; g_main_context_unref (context); return TRUE; } static gboolean foo2 (gpointer data) { return TRUE; } int main (int argc, char *argv[]) { GMainContext *context; GMainLoop *loop; context = g_main_context_default (); g_idle_add (foo1, context); g_idle_add (foo2, context); loop = g_main_loop_new (NULL, FALSE); g_main_loop_run (loop); return 0; }
Created attachment 258077 [details] [review] GMainContext: unref pending sources on destroy It is possible (but unlikely) that there will be a non-empty list of pending dispatches when we remove the last ref from a GMainContext. Make sure we drop the refs on the sources appropriately. Add a (now-working) testcase that demonstrates how to trigger the issue.
Attachment 258077 [details] pushed as 8f6be40 - GMainContext: unref pending sources on destroy