Don't guarantee that on_name_lost() will be called after g_bus_unown()

This guarantee made it impossible to use g_bus_own_name() from within
a GObject e.g. by called g_bus_own_name() in constructed() and
g_bus_unown_name() in finalize() because on_name_lost() was delivered
in an idle.

Also don't push callbacks to idle if we are already in the right
thread.

2 files changed, 42 insertions, 81 deletions

diff --git a/gdbus/gdbusnameowning.c b/gdbus/gdbusnameowning.c
index dd1020f..b174c26 100644
--- a/gdbus/gdbusnameowning.c
+++ b/gdbus/gdbusnameowning.c
@@ -113,29 +113,6 @@ client_unref (Client *client)
     }
 }
 
-static gboolean
-schedule_unref_in_idle_cb (gpointer data)
-{
-  Client *client = data;
-  client_unref (client);
-  return FALSE;
-}
-
-static void
-schedule_unref_in_idle (Client *client)
-{
-  GSource *idle_source;
-
-  idle_source = g_idle_source_new ();
-  g_source_set_priority (idle_source, G_PRIORITY_HIGH);
-  g_source_set_callback (idle_source,
-                         schedule_unref_in_idle_cb,
-                         client_ref (client),
-                         (GDestroyNotify) client_unref);
-  g_source_attach (idle_source, client->main_context);
-  g_source_unref (idle_source);
-}
-
 /* ---------------------------------------------------------------------------------------------------- */
 
 
@@ -167,28 +144,26 @@ call_handler_data_free (CallHandlerData *data)
   g_free (data);
 }
 
-static gboolean
-call_in_idle_cb (gpointer _data)
+static void
+actually_do_call (Client *client, GDBusConnection *connection, CallType call_type)
 {
-  CallHandlerData *data = _data;
-
-  switch (data->call_type)
+  switch (call_type)
     {
     case CALL_TYPE_NAME_ACQUIRED:
-      if (data->client->name_acquired_handler != NULL)
+      if (client->name_acquired_handler != NULL)
         {
-          data->client->name_acquired_handler (data->connection,
-                                               data->client->name,
-                                               data->client->user_data);
+          client->name_acquired_handler (connection,
+                                         client->name,
+                                         client->user_data);
         }
       break;
 
     case CALL_TYPE_NAME_LOST:
-      if (data->client->name_lost_handler != NULL)
+      if (client->name_lost_handler != NULL)
         {
-          data->client->name_lost_handler (data->connection,
-                                           data->client->name,
-                                           data->client->user_data);
+          client->name_lost_handler (connection,
+                                     client->name,
+                                     client->user_data);
         }
       break;
 
@@ -196,13 +171,18 @@ call_in_idle_cb (gpointer _data)
       g_assert_not_reached ();
       break;
     }
+}
 
+static gboolean
+call_in_idle_cb (gpointer _data)
+{
+  CallHandlerData *data = _data;
+  actually_do_call (data->client, data->connection, data->call_type);
   return FALSE;
 }
 
 static void
-schedule_call_in_idle (Client   *client,
-                       CallType  call_type)
+schedule_call_in_idle (Client *client, CallType  call_type)
 {
   CallHandlerData *data;
   GSource *idle_source;
@@ -223,6 +203,16 @@ schedule_call_in_idle (Client   *client,
 }
 
 static void
+do_call (Client *client, CallType call_type)
+{
+  /* only schedule in idle if we're not in the right thread */
+  if (g_main_context_get_thread_default () != client->main_context)
+    schedule_call_in_idle (client, call_type);
+  else
+    actually_do_call (client, client->connection, call_type);
+}
+
+static void
 call_acquired_handler (Client *client)
 {
   if (client->previous_call != PREVIOUS_CALL_ACQUIRED)
@@ -230,21 +220,20 @@ call_acquired_handler (Client *client)
       client->previous_call = PREVIOUS_CALL_ACQUIRED;
       if (!client->cancelled)
         {
-          schedule_call_in_idle (client, CALL_TYPE_NAME_ACQUIRED);
+          do_call (client, CALL_TYPE_NAME_ACQUIRED);
         }
     }
 }
 
 static void
-call_lost_handler (Client  *client,
-                   gboolean ignore_cancelled)
+call_lost_handler (Client  *client)
 {
   if (client->previous_call != PREVIOUS_CALL_LOST)
     {
       client->previous_call = PREVIOUS_CALL_LOST;
-      if ((!client->cancelled) || ignore_cancelled)
+      if (!client->cancelled)
         {
-          schedule_call_in_idle (client, CALL_TYPE_NAME_LOST);
+          do_call (client, CALL_TYPE_NAME_LOST);
         }
     }
 }
@@ -273,7 +262,7 @@ on_name_lost_or_acquired (GDBusConnection  *connection,
       g_variant_get (parameters, "(s)", &name);
       if (g_strcmp0 (name, client->name) == 0)
         {
-          call_lost_handler (client, FALSE);
+          call_lost_handler (client);
         }
     }
   else if (g_strcmp0 (signal_name, "NameAcquired") == 0)
@@ -325,7 +314,7 @@ request_name_cb (GObject      *source_object,
 
     case 2: /* DBUS_REQUEST_NAME_REPLY_IN_QUEUE */
       /* Waiting in line - listen for NameLost and NameAcquired */
-      call_lost_handler (client, FALSE);
+      call_lost_handler (client);
       subscribe = TRUE;
       client->needs_release = TRUE;
       break;
@@ -335,7 +324,7 @@ request_name_cb (GObject      *source_object,
     case 3: /* DBUS_REQUEST_NAME_REPLY_EXISTS */
     case 4: /* DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER */
       /* Some other part of the process is already owning the name */
-      call_lost_handler (client, FALSE);
+      call_lost_handler (client);
       break;
     }
 
@@ -389,7 +378,7 @@ on_connection_disconnected (GDBusConnection *connection,
   client->name_lost_subscription_id = 0;
   client->connection = NULL;
 
-  call_lost_handler (client, FALSE);
+  call_lost_handler (client);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
@@ -429,7 +418,7 @@ connection_get_cb (GObject      *source_object,
   client->connection = g_bus_get_finish (res, NULL);
   if (client->connection == NULL)
     {
-      call_lost_handler (client, FALSE);
+      call_lost_handler (client);
       goto out;
     }
 
@@ -641,10 +630,7 @@ g_bus_own_name (GBusType                  bus_type,
  * @owner_id: An identifier obtained from g_bus_own_name()
  *
  * Stops owning a name.
- *
- * If currently owning the name (e.g. @name_acquired_handler was the
- * last handler to be invoked), then @name_lost_handler will be invoked.
- **/
+ */
 void
 g_bus_unown_name (guint owner_id)
 {
@@ -706,12 +692,6 @@ g_bus_unown_name (guint owner_id)
                 }
               g_variant_unref (result);
             }
-
-          call_lost_handler (client, TRUE);
-        }
-      else
-        {
-          call_lost_handler (client, TRUE);
         }
 
       if (client->disconnected_signal_handler_id > 0)
@@ -729,6 +709,6 @@ g_bus_unown_name (guint owner_id)
           client->connection = NULL;
         }
 
-      schedule_unref_in_idle (client);
+      client_unref (client);
     }
 }
diff --git a/gdbus/tests/names.c b/gdbus/tests/names.c
index ac338d7..80c4650 100644
--- a/gdbus/tests/names.c
+++ b/gdbus/tests/names.c
@@ -134,7 +134,6 @@ test_bus_own_name (void)
   g_bus_unown_name (id);
   g_assert_cmpint (data.num_acquired, ==, 0);
   g_assert_cmpint (data.num_lost,     ==, 1);
-  g_main_loop_run (loop);
   g_assert_cmpint (data.num_free_func, ==, 1);
 
   /**
@@ -187,14 +186,9 @@ test_bus_own_name (void)
   g_variant_unref (result);
 
   /**
-   * Stop owning the name - this should trigger name_lost_handler()
-   * (in an idle handler) from g_bus_unown_name().
+   * Stop owning the name - this should invoke our free func
    */
-  data.expect_null_connection = FALSE;
   g_bus_unown_name (id);
-  g_main_loop_run (loop);
-  g_assert_cmpint (data.num_acquired, ==, 1);
-  g_assert_cmpint (data.num_lost,     ==, 1);
   g_assert_cmpint (data.num_free_func, ==, 2);
 
   /**
@@ -274,7 +268,6 @@ test_bus_own_name (void)
   g_assert_cmpint (data2.num_bus_acquired, ==, 1);
   g_assert_cmpint (data2.num_acquired, ==, 0);
   g_assert_cmpint (data2.num_lost,     ==, 1);
-  g_main_loop_run (loop);
   g_assert_cmpint (data2.num_free_func, ==, 1);
 
   /**
@@ -309,7 +302,6 @@ test_bus_own_name (void)
   g_assert_cmpint (data2.num_bus_acquired, ==, 0);
   g_assert_cmpint (data2.num_acquired, ==, 0);
   g_assert_cmpint (data2.num_lost,     ==, 1);
-  g_main_loop_run (loop);
   g_assert_cmpint (data2.num_free_func, ==, 1);
   /* then with _REPLACE */
   data2.num_bus_acquired = 0;
@@ -335,7 +327,6 @@ test_bus_own_name (void)
   g_assert_cmpint (data2.num_bus_acquired, ==, 0);
   g_assert_cmpint (data2.num_acquired, ==, 0);
   g_assert_cmpint (data2.num_lost,     ==, 1);
-  g_main_loop_run (loop);
   g_assert_cmpint (data2.num_free_func, ==, 1);
 
   /**
@@ -343,10 +334,8 @@ test_bus_own_name (void)
    */
   data.expect_null_connection = FALSE;
   g_bus_unown_name (id);
-  g_main_loop_run (loop);
   g_assert_cmpint (data.num_bus_acquired, ==, 1);
   g_assert_cmpint (data.num_acquired, ==, 1);
-  g_assert_cmpint (data.num_lost,     ==, 1);
   g_assert_cmpint (data.num_free_func, ==, 3);
   /* grab it again */
   data.num_bus_acquired = 0;
@@ -401,7 +390,6 @@ test_bus_own_name (void)
   g_assert_cmpint (data2.num_bus_acquired, ==, 0);
   g_assert_cmpint (data2.num_acquired, ==, 0);
   g_assert_cmpint (data2.num_lost,     ==, 1);
-  g_main_loop_run (loop);
   g_assert_cmpint (data2.num_free_func, ==, 1);
   /* then with _REPLACE - here we should acquire the name - e.g. owner should lose it
    * and owner2 should acquire it  */
@@ -431,11 +419,8 @@ test_bus_own_name (void)
   g_assert_cmpint (data2.num_bus_acquired, ==, 0);
   /* ok, make owner2 release the name - then wait for owner to automagically reacquire it */
   g_bus_unown_name (id2);
-  g_main_loop_run (loop);
-  g_assert_cmpint (data2.num_acquired, ==, 1);
-  g_assert_cmpint (data2.num_lost,     ==, 1);
-  g_main_loop_run (loop);
   g_assert_cmpint (data2.num_free_func, ==, 1);
+  g_main_loop_run (loop);
   g_assert_cmpint (data.num_acquired, ==, 2);
   g_assert_cmpint (data.num_lost,     ==, 1);
 
@@ -450,8 +435,6 @@ test_bus_own_name (void)
   g_assert_cmpint (data.num_acquired, ==, 2);
   g_assert_cmpint (data.num_lost,     ==, 2);
   g_bus_unown_name (id);
-  while (data.num_free_func != 4)
-    g_main_loop_run (loop);
   g_assert_cmpint (data.num_free_func, ==, 4);
 
   g_object_unref (c);
@@ -631,9 +614,7 @@ test_bus_watch_name (void)
 
   /* unown the name */
   g_bus_unown_name (owner_id);
-  g_main_loop_run (loop);
   g_assert_cmpint (data.num_acquired, ==, 1);
-  g_assert_cmpint (data.num_lost,     ==, 1);
   g_assert_cmpint (data.num_free_func, ==, 2);
 
   /**
@@ -683,6 +664,7 @@ test_bus_watch_name (void)
   session_bus_down ();
   g_main_loop_run (loop);
   g_assert_cmpint (data.num_lost,     ==, 1);
+  g_main_loop_run (loop);
   g_assert_cmpint (data.num_vanished, ==, 2);
 
   g_bus_unwatch_name (id);
@@ -690,7 +672,6 @@ test_bus_watch_name (void)
   g_assert_cmpint (data.num_free_func, ==, 1);
 
   g_bus_unown_name (owner_id);
-  g_main_loop_run (loop);
   g_assert_cmpint (data.num_free_func, ==, 2);
 
 }