From: INSUN PYO Date: Thu, 29 Dec 2016 02:26:43 +0000 (+0900) Subject: gdbus:fix race condition between signal callback and g_bus_unwatch_name function X-Git-Tag: accepted/tizen/common/20170201.171637~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a0dc33df372eb7a54a89221b97db753d560fcfd7;p=platform%2Fupstream%2Fglib.git gdbus:fix race condition between signal callback and g_bus_unwatch_name function Signed-off-by: INSUN PYO Change-Id: Ia19381a866674a3343503dab7feab0950bb49238 (cherry picked from commit 9002008efdce0bbf351dae5338fbe782369c3e20) --- diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index 628cef8..2193ea6 100755 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -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; diff --git a/gio/gdbusconnection.h b/gio/gdbusconnection.h index e5ef808..2031c2f 100644 --- a/gio/gdbusconnection.h +++ b/gio/gdbusconnection.h @@ -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); diff --git a/gio/gdbusnamewatching.c b/gio/gdbusnamewatching.c index 668134f..d287c1a 100755 --- a/gio/gdbusnamewatching.c +++ b/gio/gdbusnamewatching.c @@ -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) {