Don't guarantee that on_name_vanished() will be called after g_bus_unwatch()

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

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

2 files changed, 49 insertions, 65 deletions

diff --git a/gdbus/gdbusnamewatching.c b/gdbus/gdbusnamewatching.c
index dbaad80..cc8a8eb 100644
--- a/gdbus/gdbusnamewatching.c
+++ b/gdbus/gdbusnamewatching.c
@@ -108,30 +108,13 @@ 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)
+typedef enum
 {
-  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);
-}
-
-/* ---------------------------------------------------------------------------------------------------- */
+  CALL_TYPE_NAME_APPEARED,
+  CALL_TYPE_NAME_VANISHED
+} CallType;
 
 typedef struct
 {
@@ -145,8 +128,7 @@ typedef struct
   /* ditto */
   gchar *name_owner;
 
-  /* set to TRUE to call appeared */
-  gboolean call_appeared;
+  CallType call_type;
 } CallHandlerData;
 
 static void
@@ -159,37 +141,46 @@ 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, const gchar *name_owner, CallType call_type)
 {
-  CallHandlerData *data = _data;
-
-  if (data->call_appeared)
+  switch (call_type)
     {
-      if (data->client->name_appeared_handler != NULL)
+    case CALL_TYPE_NAME_APPEARED:
+      if (client->name_appeared_handler != NULL)
         {
-          data->client->name_appeared_handler (data->connection,
-                                               data->client->name,
-                                               data->name_owner,
-                                               data->client->user_data);
+          client->name_appeared_handler (connection,
+                                         client->name,
+                                         name_owner,
+                                         client->user_data);
         }
-    }
-  else
-    {
-      if (data->client->name_vanished_handler != NULL)
+      break;
+
+    case CALL_TYPE_NAME_VANISHED:
+      if (client->name_vanished_handler != NULL)
         {
-          data->client->name_vanished_handler (data->connection,
-                                               data->client->name,
-                                               data->client->user_data);
+          client->name_vanished_handler (connection,
+                                         client->name,
+                                         client->user_data);
         }
+      break;
+
+    default:
+      g_assert_not_reached ();
+      break;
     }
+}
 
+static gboolean
+call_in_idle_cb (gpointer _data)
+{
+  CallHandlerData *data = _data;
+  actually_do_call (data->client, data->connection, data->name_owner, data->call_type);
   return FALSE;
 }
 
 static void
-schedule_call_in_idle (Client *client,
-                       gboolean call_appeared)
+schedule_call_in_idle (Client *client, CallType call_type)
 {
   CallHandlerData *data;
   GSource *idle_source;
@@ -198,7 +189,7 @@ schedule_call_in_idle (Client *client,
   data->client = client_ref (client);
   data->connection = client->connection != NULL ? g_object_ref (client->connection) : NULL;
   data->name_owner = g_strdup (client->name_owner);
-  data->call_appeared = call_appeared;
+  data->call_type = call_type;
 
   idle_source = g_idle_source_new ();
   g_source_set_priority (idle_source, G_PRIORITY_HIGH);
@@ -211,6 +202,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, client->name_owner, call_type);
+}
+
+static void
 call_appeared_handler (Client *client)
 {
   if (client->previous_call != PREVIOUS_CALL_APPEARED)
@@ -218,7 +219,7 @@ call_appeared_handler (Client *client)
       client->previous_call = PREVIOUS_CALL_APPEARED;
       if (!client->cancelled && client->name_appeared_handler != NULL)
         {
-          schedule_call_in_idle (client, TRUE);
+          do_call (client, CALL_TYPE_NAME_APPEARED);
         }
     }
 }
@@ -232,7 +233,7 @@ call_vanished_handler (Client  *client,
       client->previous_call = PREVIOUS_CALL_VANISHED;
       if (((!client->cancelled) || ignore_cancelled) && client->name_vanished_handler != NULL)
         {
-          schedule_call_in_idle (client, FALSE);
+          do_call (client, CALL_TYPE_NAME_VANISHED);
         }
     }
 }
@@ -504,9 +505,6 @@ g_bus_watch_name (GBusType                  bus_type,
  * @watcher_id: An identifier obtained from g_bus_watch_name()
  *
  * Stops watching a name.
- *
- * If the name being watched currently has an owner the name (e.g. @name_appeared_handler
- * was the last handler to be invoked), then @name_vanished_handler will be invoked.
  **/
 void
 g_bus_unwatch_name (guint watcher_id)
@@ -533,7 +531,6 @@ g_bus_unwatch_name (guint watcher_id)
   /* do callback without holding lock */
   if (client != NULL)
     {
-      call_vanished_handler (client, TRUE);
-      schedule_unref_in_idle (client);
+      client_unref (client);
     }
 }
diff --git a/gdbus/tests/names.c b/gdbus/tests/names.c
index 80c4650..f779264 100644
--- a/gdbus/tests/names.c
+++ b/gdbus/tests/names.c
@@ -558,7 +558,6 @@ test_bus_watch_name (void)
   g_bus_unwatch_name (id);
   g_assert_cmpint (data.num_appeared, ==, 0);
   g_assert_cmpint (data.num_vanished, ==, 1);
-  g_main_loop_run (loop);
   g_assert_cmpint (data.num_free_func, ==, 1);
 
   /**
@@ -597,19 +596,9 @@ test_bus_watch_name (void)
   g_assert_cmpint (data.num_vanished, ==, 0);
 
   /**
-   * Unwatch the name - this should trigger name_vanished_handler() because of this
-   * guarantee
-   *
-   *   If the name being watched currently has an owner the name (e.g. @name_appeared_handler
-   *   was the last handler to be invoked), then @name_vanished_handler will be invoked
-   *   before this function returns.
-   *
-   * in g_bus_unwatch_name().
+   * Unwatch the name.
    */
   g_bus_unwatch_name (id);
-  g_main_loop_run (loop);
-  g_assert_cmpint (data.num_appeared, ==, 1);
-  g_assert_cmpint (data.num_vanished, ==, 1);
   g_assert_cmpint (data.num_free_func, ==, 1);
 
   /* unown the name */
@@ -664,11 +653,9 @@ 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);
-  g_main_loop_run (loop);
   g_assert_cmpint (data.num_free_func, ==, 1);
 
   g_bus_unown_name (owner_id);