From dba4e8ece893b9722ba4d0df03889471456cc318 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Mon, 9 Mar 2015 17:13:30 +0100 Subject: [PATCH] libnm,nm-object: fix tracing of object removal When a new connection is activated and presently active connection goes away, the active-connection-removed signal is not emitted for the old connection. This is what happens: 1.) Initially, nm-manager::active-connections = [ActiveConnection/old] 2.) First PropertyChange is signalled for the new connection addition: nm-manager::active-connections = [ActiveConnection/old,ActiveConnection/new] This triggers load of ActiveConnection/new object. 3.) Another PropertyChange is signalled for the old connection removal: nm-manager::active-connections = [ActiveConnection/new] This removes the ActiveConnection/old object from nm-manager::active-connections and enqueues active-connection-removed signal. The signal is not emmitted as there's a reload from 2.) in progress. 4.) ActiveConnection/new reload finished object_property_complete() compares [ActiveConnection/old,ActiveConnection/new] from its odata to current nm-manager::active-connections and incorrectly concludes that ActiveConnection/old was just added and removes the enqueued active-connection-removed signal. This patch fixes the issue by remembering the original nm-manager::active-connections property value at 2.). [thaller@redhat.com: fixed an integer overflow and odata->array unreffing] https://bugzilla.redhat.com/show_bug.cgi?id=1079353 --- libnm/nm-object.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/libnm/nm-object.c b/libnm/nm-object.c index 391166b29..fbdbc1f62 100644 --- a/libnm/nm-object.c +++ b/libnm/nm-object.c @@ -625,7 +625,7 @@ typedef struct { GObject **objects; int length, remaining; - gboolean array; + GPtrArray *array; const char *property_name; } ObjectCreatedData; @@ -671,7 +671,8 @@ object_property_complete (ObjectCreatedData *odata) gboolean different = TRUE; if (odata->array) { - GPtrArray *old = *((GPtrArray **) pi->field); + GPtrArray *pi_old = *((GPtrArray **) pi->field); + GPtrArray *old = odata->array; GPtrArray *new; int i; @@ -684,16 +685,11 @@ object_property_complete (ObjectCreatedData *odata) GPtrArray *added = g_ptr_array_sized_new (3); GPtrArray *removed = g_ptr_array_sized_new (3); - if (old) { - /* Find objects in 'old' that do not exist in 'new' */ - array_diff (old, new, removed); + /* Find objects in 'old' that do not exist in 'new' */ + array_diff (old, new, removed); - /* Find objects in 'new' that do not exist in old */ - array_diff (new, old, added); - } else { - for (i = 0; i < new->len; i++) - g_ptr_array_add (added, g_ptr_array_index (new, i)); - } + /* Find objects in 'new' that do not exist in old */ + array_diff (new, old, added); *((GPtrArray **) pi->field) = new; @@ -726,8 +722,8 @@ object_property_complete (ObjectCreatedData *odata) /* Free old array last since it will release references, thus freeing * any objects in the 'removed' array. */ - if (old) - g_ptr_array_unref (old); + if (pi_old) + g_ptr_array_unref (pi_old); } else { GObject **obj_p = pi->field; @@ -745,6 +741,8 @@ object_property_complete (ObjectCreatedData *odata) g_object_unref (self); g_free (odata->objects); + if (odata->array) + g_ptr_array_unref (odata->array); g_slice_free (ObjectCreatedData, odata); } @@ -781,7 +779,7 @@ handle_object_property (NMObject *self, const char *property_name, GVariant *val odata->pi = pi; odata->objects = g_new (GObject *, 1); odata->length = odata->remaining = 1; - odata->array = FALSE; + odata->array = NULL; odata->property_name = property_name; priv->reload_remaining++; @@ -820,6 +818,7 @@ handle_object_array_property (NMObject *self, const char *property_name, GVarian GPtrArray **array = pi->field; const char *path; ObjectCreatedData *odata; + guint i, len = *array ? (*array)->len : 0; npaths = g_variant_n_children (value); @@ -828,9 +827,13 @@ handle_object_array_property (NMObject *self, const char *property_name, GVarian odata->pi = pi; odata->objects = g_new0 (GObject *, npaths); odata->length = odata->remaining = npaths; - odata->array = TRUE; odata->property_name = property_name; + /* Objects known at this point. */ + odata->array = g_ptr_array_new_full (len, g_object_unref); + for (i = 0; i < len; i++) + g_ptr_array_add (odata->array, g_object_ref (g_ptr_array_index (*array, i))); + priv->reload_remaining++; if (npaths == 0) {