From ed009b4d497e748f43887e91f4f777c6cdb5e43d Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Tue, 20 Jan 2015 18:40:18 +0100
Subject: [PATCH 1/2] Make the ref / unref pairs more obvious
This makes the code more readable and robust against refactorings.
https://bugzilla.gnome.org/show_bug.cgi?id=741257
---
libgupnp/gupnp-network-manager.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/libgupnp/gupnp-network-manager.c b/libgupnp/gupnp-network-manager.c
index f9c304e7f5df..0d53291b0479 100644
--- a/libgupnp/gupnp-network-manager.c
+++ b/libgupnp/gupnp-network-manager.c
@@ -163,7 +163,6 @@ nm_device_unref (NMDevice *nm_device)
g_object_unref (nm_device->context);
}
- g_object_unref (nm_device->proxy);
g_object_unref (nm_device->manager);
g_slice_free (NMDevice, nm_device);
@@ -379,7 +378,7 @@ use_new_device (GUPnPNetworkManager *manager,
GVariant *value;
manager->priv->nm_devices = g_list_append (manager->priv->nm_devices,
- nm_device);
+ nm_device_ref (nm_device));
g_signal_connect (nm_device->proxy,
"g-signal",
@@ -433,7 +432,7 @@ device_proxy_new_cb (GObject *source_object,
gpointer user_data) {
GUPnPNetworkManager *manager;
GDBusProxy *device_proxy;
- NMDevice *nm_device;
+ NMDevice *nm_device = NULL;
NMDeviceType type;
GVariant *value;
GError *error;
@@ -451,14 +450,11 @@ device_proxy_new_cb (GObject *source_object,
value = g_dbus_proxy_get_cached_property (device_proxy, "DeviceType");
if (G_UNLIKELY (value == NULL)) {
- g_object_unref (device_proxy);
-
goto done;
}
if (G_UNLIKELY (!g_variant_is_of_type (value, G_VARIANT_TYPE_UINT32))) {
g_variant_unref (value);
- g_object_unref (device_proxy);
goto done;
}
@@ -485,6 +481,8 @@ device_proxy_new_cb (GObject *source_object,
use_new_device (manager, nm_device);
done:
+ g_clear_pointer (&nm_device, (GDestroyNotify) nm_device_unref);
+ g_clear_object (&device_proxy);
g_object_unref (manager);
}
--
2.5.0
From d2e0dc2a8fdb104950f01f153ff60eb663de7a88 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Tue, 20 Jan 2015 18:40:38 +0100
Subject: [PATCH 2/2] Use the GCancellable to cancel pending operations during
destruction
Relying on references to stay alive across idle and asynchronous calls
confuses users because the object stays alive even after the user has
dropped the only reference known to it. This can lead to crashes due
to invalid memory access if the object or its children emit signals
after the user has itself been destructed.
https://bugzilla.gnome.org/show_bug.cgi?id=741257
---
libgupnp/gupnp-network-manager.c | 48 ++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 22 deletions(-)
diff --git a/libgupnp/gupnp-network-manager.c b/libgupnp/gupnp-network-manager.c
index 0d53291b0479..1111268e3db6 100644
--- a/libgupnp/gupnp-network-manager.c
+++ b/libgupnp/gupnp-network-manager.c
@@ -129,7 +129,7 @@ nm_device_new (GUPnPNetworkManager *manager,
nm_device = g_slice_new0 (NMDevice);
g_atomic_int_set (&nm_device->ref_count, 1);
- nm_device->manager = g_object_ref (manager);
+ nm_device->manager = manager;
nm_device->proxy = g_object_ref (device_proxy);
return nm_device;
@@ -155,7 +155,7 @@ nm_device_unref (NMDevice *nm_device)
if (nm_device->ap_proxy != NULL)
g_object_unref (nm_device->ap_proxy);
- if (nm_device->context != NULL) {
+ if (nm_device->manager != NULL && nm_device->context != NULL) {
g_signal_emit_by_name (nm_device->manager,
"context-unavailable",
nm_device->context);
@@ -163,8 +163,6 @@ nm_device_unref (NMDevice *nm_device)
g_object_unref (nm_device->context);
}
- g_object_unref (nm_device->manager);
-
g_slice_free (NMDevice, nm_device);
}
@@ -279,7 +277,10 @@ ap_proxy_new_cb (GObject *source_object,
nm_device->ap_proxy = g_dbus_proxy_new_for_bus_finish (res, &error);
if (G_UNLIKELY (error != NULL)) {
- g_message ("Failed to create D-Bus proxy: %s", error->message);
+ if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ g_message ("Failed to create D-Bus proxy: %s", error->message);
+ else
+ nm_device->manager = NULL;
g_error_free (error);
goto done;
}
@@ -415,7 +416,10 @@ wifi_proxy_new_cb (GObject *source_object,
nm_device->wifi_proxy = g_dbus_proxy_new_for_bus_finish (res, &error);
if (G_UNLIKELY (error != NULL)) {
- g_message ("Failed to create D-Bus proxy: %s", error->message);
+ if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ g_message ("Failed to create D-Bus proxy: %s", error->message);
+ else
+ nm_device->manager = NULL;
g_error_free (error);
goto done;
}
@@ -437,17 +441,19 @@ device_proxy_new_cb (GObject *source_object,
GVariant *value;
GError *error;
- manager = GUPNP_NETWORK_MANAGER (user_data);
error = NULL;
device_proxy = g_dbus_proxy_new_for_bus_finish (res, &error);
if (G_UNLIKELY (error != NULL)) {
- g_message ("Failed to create D-Bus proxy: %s", error->message);
+ if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ g_message ("Failed to create D-Bus proxy: %s", error->message);
g_error_free (error);
goto done;
}
+ manager = GUPNP_NETWORK_MANAGER (user_data);
+
value = g_dbus_proxy_get_cached_property (device_proxy, "DeviceType");
if (G_UNLIKELY (value == NULL)) {
goto done;
@@ -483,7 +489,6 @@ device_proxy_new_cb (GObject *source_object,
done:
g_clear_pointer (&nm_device, (GDestroyNotify) nm_device_unref);
g_clear_object (&device_proxy);
- g_object_unref (manager);
}
static int
@@ -525,7 +530,7 @@ on_manager_signal (GDBusProxy *proxy,
DEVICE_INTERFACE,
manager->priv->cancellable,
device_proxy_new_cb,
- g_object_ref (manager));
+ manager);
g_free (device_path);
} else if (g_strcmp0 (signal_name, "DeviceRemoved") == 0) {
GList *device_node;
@@ -568,19 +573,20 @@ get_devices_cb (GObject *source_object,
char* device_path;
GError *error = NULL;
- manager = GUPNP_NETWORK_MANAGER (user_data);
-
ret = g_dbus_proxy_call_finish (G_DBUS_PROXY (source_object),
res,
&error);
if (error != NULL) {
- g_warning ("Error fetching list of devices: %s",
- error->message);
+ if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+ g_warning ("Error fetching list of devices: %s",
+ error->message);
g_error_free (error);
- goto done;
+ return;
}
+ manager = GUPNP_NETWORK_MANAGER (user_data);
+
g_variant_get_child (ret, 0, "ao", &device_iter);
while (g_variant_iter_loop (device_iter, "o", &device_path))
g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM,
@@ -591,13 +597,10 @@ get_devices_cb (GObject *source_object,
DEVICE_INTERFACE,
manager->priv->cancellable,
device_proxy_new_cb,
- g_object_ref (user_data));
+ user_data);
g_variant_iter_free (device_iter);
g_variant_unref (ret);
-
-done:
- g_object_unref (manager);
}
static void
@@ -611,8 +614,8 @@ schedule_loopback_context_creation (GUPnPNetworkManager *manager)
g_main_context_get_thread_default ());
g_source_set_callback (manager->priv->idle_context_creation_src,
create_loopback_context,
- g_object_ref (manager),
- (GDestroyNotify) g_object_unref);
+ manager,
+ NULL);
g_source_unref (manager->priv->idle_context_creation_src);
}
@@ -655,7 +658,7 @@ init_network_manager (GUPnPNetworkManager *manager)
-1,
priv->cancellable,
get_devices_cb,
- g_object_ref (manager));
+ manager);
}
static void
@@ -715,6 +718,7 @@ gupnp_network_manager_dispose (GObject *object)
g_list_free_full (priv->nm_devices, (GDestroyNotify) nm_device_unref);
if (priv->cancellable != NULL) {
+ g_cancellable_cancel (priv->cancellable);
g_object_unref (priv->cancellable);
priv->cancellable = NULL;
}
--
2.5.0