GDBus: Unref worker from worker-thread to avoid race

... otherwise we might end up using the worker after it has been
freed. Reported by Dan Winship and Colin Walters.

This fix uncovered a bug in the /gdbus/nonce-tcp test case so "fix"
that as well to use a better way of having one thread wait for another
(using quotes for the word "fix" since it's pretty hackish to
busy-wait in one thread to wait for another).

Signed-off-by: David Zeuthen <davidz@redhat.com>

diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c
index fcc8cf788be332d5484ec7afcf8f7b754fe78d56..aa2a16e8101026af98bcee96b8b09612c8693ac7 100644 (file)
--- a/gio/gdbusprivate.c
+++ b/gio/gdbusprivate.c
@@ -1401,6 +1401,15 @@ _g_dbus_worker_new (GIOStream                              *stream,
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+/* called in private thread shared by all GDBusConnection instances (without read-lock held) */
+static gboolean
+unref_in_idle_cb (gpointer user_data)
+{
+  GDBusWorker *worker = user_data;
+  _g_dbus_worker_unref (worker);
+  return FALSE;
+}
+
 /* This can be called from any thread - frees worker. Note that
  * callbacks might still happen if called from another thread than the
  * worker - use your own synchronization primitive in the callbacks.
@@ -1408,9 +1417,19 @@ _g_dbus_worker_new (GIOStream                              *stream,
 void
 _g_dbus_worker_stop (GDBusWorker *worker)
 {
+  GSource *idle_source;
+
   worker->stopped = TRUE;
   g_cancellable_cancel (worker->cancellable);
-  _g_dbus_worker_unref (worker);
+
+  idle_source = g_idle_source_new ();
+  g_source_set_priority (idle_source, G_PRIORITY_DEFAULT);
+  g_source_set_callback (idle_source,
+                         unref_in_idle_cb,
+                         _g_dbus_worker_ref (worker),
+                         (GDestroyNotify) _g_dbus_worker_unref);
+  g_source_attach (idle_source, worker->shared_thread_data->context);
+  g_source_unref (idle_source);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */

diff --git a/gio/tests/gdbus-peer.c b/gio/tests/gdbus-peer.c
index 3fba622c3dd2f38c888067f4f93a73e42ff03439..c71a526989f060bca7bf89d588d75e964dfcce1a 100644 (file)
--- a/gio/tests/gdbus-peer.c
+++ b/gio/tests/gdbus-peer.c
@@ -1206,7 +1206,7 @@ test_nonce_tcp (void)
   g_assert_no_error (error);
   g_assert (c != NULL);
   while (data.current_connections->len < 1)
-    g_main_loop_run (loop);
+    g_thread_yield ();
   g_assert_cmpint (data.current_connections->len, ==, 1);
   g_assert_cmpint (data.num_connection_attempts, ==, 1);
   g_assert (g_dbus_connection_get_unique_name (c) == NULL);