rpms/gupnp
6
0
Fork 0
Code Issues Pull Requests Releases Activity

GUPnPNetworkManager is never deallocated

Browse Source 
... due to internal circular reference.

https://bugzilla.gnome.org/show_bug.cgi?id=741257
This commit is contained in:
Debarshi Ray 2015-12-14 12:32:09 +01:00
parent c15c6b3b71
commit 20573c8c99
2 changed files with 262 additions and 1 deletions

253
gupnp-GUPnPNetworkManager-is-never-deallocated.patch Normal file
View File

@ -0,0 +1,253 @@
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

10
gupnp.spec
View File

@ -1,6 +1,6 @@
Name: gupnp
Version: 0.20.14
Release: 3%{?dist}
Release: 4%{?dist}
Summary: A framework for creating UPnP devices & control points
Group: System Environment/Libraries
@ -8,6 +8,9 @@ License: LGPLv2+
URL: http://www.gupnp.org/
Source0: http://download.gnome.org/sources/%{name}/0.20/%{name}-%{version}.tar.xz
# https://bugzilla.gnome.org/show_bug.cgi?id=741257
Patch0: gupnp-GUPnPNetworkManager-is-never-deallocated.patch
BuildRequires: gssdp-devel >= 0.14.0
BuildRequires: gtk-doc
BuildRequires: gobject-introspection-devel >= 1.36
@ -50,6 +53,7 @@ This package contains developer documentation for %{name}.
%prep
%setup -q
%patch0 -p1
%build
%configure --disable-static --with-context-manager=network-manager
@ -89,6 +93,10 @@ make check %{?_smp_mflags} V=1
%doc %{_datadir}/gtk-doc/html/%{name}
%changelog
* Mon Dec 14 2015 Debarshi Ray <rishi@fedoraproject.org> - 0.20.14-4
- GUPnPNetworkManager is never deallocated due to internal circular reference
(GNOME #741257)
* Fri Jul 03 2015 Kalev Lember <klember@redhat.com> - 0.20.14-3
- Switch to Python 3 (#1192093)
- Move gupnp-binding-tool to -devel subpackage