gdbus:fix race condition between signal callback and g_bus_unwatch_name function 12/110212/1
authorINSUN PYO <insun.pyo@samsung.com>
Thu, 29 Dec 2016 02:26:43 +0000 (11:26 +0900)
committersanghyeok oh <sanghyeok.oh@samsung.com>
Fri, 13 Jan 2017 08:29:01 +0000 (00:29 -0800)
Signed-off-by: INSUN PYO <insun.pyo@samsung.com>
Change-Id: Ia19381a866674a3343503dab7feab0950bb49238
(cherry picked from commit 9002008efdce0bbf351dae5338fbe782369c3e20)

gio/gdbusconnection.c
gio/gdbusconnection.h
gio/gdbusnamewatching.c

index 628cef8..2193ea6 100755 (executable)
@@ -4111,6 +4111,8 @@ typedef struct
 {
   GDBusSignalCallback callback;
   gpointer user_data;
+  GFunc user_data_ref_func;
+  GFunc user_data_unref_func;
   GDestroyNotify user_data_free_func;
   guint id;
   GMainContext *context;
@@ -4307,6 +4309,34 @@ g_dbus_connection_signal_subscribe (GDBusConnection     *connection,
                                     gpointer             user_data,
                                     GDestroyNotify       user_data_free_func)
 {
+  return g_dbus_connection_signal_subscribe_with_ref (connection,
+                                                      sender,
+                                                      interface_name,
+                                                      member,
+                                                      object_path,
+                                                      arg0,
+                                                      flags,
+                                                      callback,
+                                                      user_data,
+                                                      NULL,
+                                                      NULL,
+                                                      user_data_free_func);
+}
+
+guint
+g_dbus_connection_signal_subscribe_with_ref (GDBusConnection             *connection,
+                                             const gchar                 *sender,
+                                             const gchar                 *interface_name,
+                                             const gchar                 *member,
+                                             const gchar                 *object_path,
+                                             const gchar                 *arg0,
+                                             GDBusSignalFlags            flags,
+                                             GDBusSignalCallback         callback,
+                                             gpointer                    user_data,
+                                             GFunc                       user_data_ref_func,
+                                             GFunc                       user_data_unref_func,
+                                             GDestroyNotify              user_data_free_func)
+{
   gchar *rule;
   SignalData *signal_data;
   SignalSubscriber subscriber;
@@ -4355,6 +4385,8 @@ g_dbus_connection_signal_subscribe (GDBusConnection     *connection,
   subscriber.user_data_free_func = user_data_free_func;
   subscriber.id = _global_subscriber_id++; /* TODO: overflow etc. */
   subscriber.context = g_main_context_ref_thread_default ();
+  subscriber.user_data_ref_func = user_data_ref_func;
+  subscriber.user_data_unref_func = user_data_unref_func;
 
   /* see if we've already have this rule */
   signal_data = g_hash_table_lookup (connection->map_rule_to_signal_data, rule);
@@ -4602,6 +4634,8 @@ typedef struct
   guint                subscription_id;
   GDBusSignalCallback  callback;
   gpointer             user_data;
+  GFunc                user_data_ref_func;
+  GFunc                user_data_unref_func;
   GDBusMessage        *message;
   GDBusConnection     *connection;
   const gchar         *sender;
@@ -4646,17 +4680,25 @@ emit_signal_instance_in_idle_cb (gpointer data)
   has_subscription = FALSE;
   if (g_hash_table_lookup (signal_instance->connection->map_id_to_signal_data,
                            GUINT_TO_POINTER (signal_instance->subscription_id)) != NULL)
-    has_subscription = TRUE;
+    {
+      if (signal_instance->user_data_ref_func)
+        signal_instance->user_data_ref_func (signal_instance->user_data, 0);
+      has_subscription = TRUE;
+    }
   CONNECTION_UNLOCK (signal_instance->connection);
 
   if (has_subscription)
-    signal_instance->callback (signal_instance->connection,
-                               signal_instance->sender,
-                               signal_instance->path,
-                               signal_instance->interface,
-                               signal_instance->member,
-                               parameters,
-                               signal_instance->user_data);
+    {
+      signal_instance->callback (signal_instance->connection,
+                                 signal_instance->sender,
+                                 signal_instance->path,
+                                 signal_instance->interface,
+                                 signal_instance->member,
+                                 parameters,
+                                 signal_instance->user_data);
+      if (signal_instance->user_data_unref_func)
+        signal_instance->user_data_unref_func (signal_instance->user_data, 0);
+    }
 
   g_variant_unref (parameters);
 
@@ -4792,6 +4834,8 @@ schedule_callbacks (GDBusConnection *connection,
           signal_instance->subscription_id = subscriber->id;
           signal_instance->callback = subscriber->callback;
           signal_instance->user_data = subscriber->user_data;
+          signal_instance->user_data_ref_func = subscriber->user_data_ref_func;
+          signal_instance->user_data_unref_func = subscriber->user_data_unref_func;
           signal_instance->message = g_object_ref (message);
           signal_instance->connection = g_object_ref (connection);
           signal_instance->sender = sender;
index e5ef808..2031c2f 100644 (file)
@@ -625,6 +625,19 @@ guint            g_dbus_connection_signal_subscribe           (GDBusConnection
                                                                gpointer             user_data,
                                                                GDestroyNotify       user_data_free_func);
 GLIB_AVAILABLE_IN_ALL
+guint            g_dbus_connection_signal_subscribe_with_ref  (GDBusConnection     *connection,
+                                                               const gchar         *sender,
+                                                               const gchar         *interface_name,
+                                                               const gchar         *member,
+                                                               const gchar         *object_path,
+                                                               const gchar         *arg0,
+                                                               GDBusSignalFlags     flags,
+                                                               GDBusSignalCallback  callback,
+                                                               gpointer             user_data,
+                                                               GFunc                user_data_ref_func,
+                                                               GFunc                user_data_unref_func,
+                                                               GDestroyNotify       user_data_free_func);
+GLIB_AVAILABLE_IN_ALL
 void             g_dbus_connection_signal_unsubscribe         (GDBusConnection     *connection,
                                                                guint                subscription_id);
 
index 668134f..d287c1a 100755 (executable)
@@ -109,6 +109,20 @@ client_unref (Client *client)
     }
 }
 
+static void
+client_ref_func (gpointer user_data, gpointer unused)
+{
+  if (user_data)
+    client_ref ((Client*)user_data);
+}
+
+static void
+client_unref_func (gpointer user_data, gpointer unused)
+{
+  if (user_data)
+    client_unref ((Client*)user_data);
+}
+
 /* ---------------------------------------------------------------------------------------------------- */
 
 typedef enum
@@ -297,10 +311,6 @@ on_name_owner_changed (GDBusConnection *connection,
   if (g_strcmp0 (name, client->name) != 0)
     goto out;
 
-  /* We need to keep our reference to client locally, because handlers below
-        may free it */
-  client_ref(client);
-
   if (old_owner != NULL && strlen (old_owner) > 0)
     {
       g_free (client->name_owner);
@@ -322,8 +332,6 @@ on_name_owner_changed (GDBusConnection *connection,
    */
   client->initialized = TRUE;
 
-  client_unref(client);
-
  out:
   ;
 }
@@ -459,16 +467,19 @@ has_connection (Client *client)
                                                              client);
 
   /* start listening to NameOwnerChanged messages immediately */
-  client->name_owner_changed_subscription_id = g_dbus_connection_signal_subscribe (client->connection,
-                                                                                   "org.freedesktop.DBus",  /* name */
-                                                                                   "org.freedesktop.DBus",  /* if */
-                                                                                   "NameOwnerChanged",      /* signal */
-                                                                                   "/org/freedesktop/DBus", /* path */
-                                                                                   client->name,
-                                                                                   G_DBUS_SIGNAL_FLAGS_NONE,
-                                                                                   on_name_owner_changed,
-                                                                                   client,
-                                                                                   NULL);
+  client->name_owner_changed_subscription_id
+    = g_dbus_connection_signal_subscribe_with_ref (client->connection,
+                                                   "org.freedesktop.DBus",  /* name */
+                                                   "org.freedesktop.DBus",  /* if */
+                                                   "NameOwnerChanged",      /* signal */
+                                                   "/org/freedesktop/DBus", /* path */
+                                                   client->name,
+                                                   G_DBUS_SIGNAL_FLAGS_NONE,
+                                                   on_name_owner_changed,
+                                                   client,
+                                                   client_ref_func,
+                                                   client_unref_func,
+                                                   NULL);
 
   if (client->flags & G_BUS_NAME_WATCHER_FLAGS_AUTO_START)
     {