GNOME Bugzilla – Bug 697595
g_main_context_unref unlocks a mutex twice
Last modified: 2013-04-10 18:20:49 UTC
The following code: /* test if adding a signal watch for different message types calls the * respective callbacks. */ GST_START_TEST (test_watch_with_custom_context) { GMainContext *ctx; GSource *source; guint num_eos = 0; guint num_app = 0; guint id, id_idle; test_bus = gst_bus_new (); ctx = g_main_context_new (); main_loop = g_main_loop_new (ctx, FALSE); source = gst_bus_create_watch (test_bus); g_source_set_callback (source, (GSourceFunc) gst_bus_async_signal_func, NULL, NULL); id = g_source_attach (source, ctx); g_source_unref (source); fail_if (id == 0); g_signal_connect (test_bus, "message::eos", (GCallback) message_func_eos, &num_eos); g_signal_connect (test_bus, "message::application", (GCallback) message_func_app, &num_app); source = g_idle_source_new (); g_source_set_callback (source, (GSourceFunc) send_messages, NULL, NULL); id_idle = g_source_attach (source, ctx); g_source_unref (source); fail_if (id_idle == 0); while (g_main_context_pending (ctx)) g_main_context_iteration (ctx, FALSE); fail_unless_equals_int (num_eos, 10); fail_unless_equals_int (num_app, 10); g_source_remove (id); g_source_remove (id_idle); g_main_loop_unref (main_loop); g_main_context_unref (ctx); gst_object_unref (test_bus); } crashes in g_main_context_unref (), because context->mutex is unlocked twice in a row: g_source_iter_init() stores the context pointer. g_source_iter_next() is then repeatedly called. g_source_iter_next() calls a SOURCE_UNREF() macro, which uses the context pointer stored earlier. SOURCE_UNREF() calls g_source_unref_internal() with that context pointer and have_lock==1. g_source_unref_internal() doesn't lock the context (have_lock==1), potentially destroys the source, and, if finalization function is to be called, temporarily unlocks the context - at which point it crashes, because it did not actually held that mutex.
Created attachment 240995 [details] [review] Proposed fix Note that simply adding locking to g_main_context_unref() does not work and leads to double lock instead (i.e. process hangs up).
Created attachment 241179 [details] [review] gmain: fix double-unlock in g_main_context_unref() When unreffing a context with sources still attached, it would end up unlocking an already-unlocked context, causing crashes on platforms that (unlike Linux) actually check for that. ==== > Note that simply adding locking to g_main_context_unref() does not work and > leads to double lock instead (i.e. process hangs up). It seems to work fine for me; I suspect you forgot to adjust the g_source_destroy_internal() call inside the loop. Does this patch work for you?
*facepalm* Yeah, your patch works. Thanks.
Review of attachment 241179 [details] [review]: Go-go gadget patch!
Attachment 241179 [details] pushed as 0513c85 - gmain: fix double-unlock in g_main_context_unref()