From 2ac83c0e8b06de9ebfce3038c1afb01061f6e680 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Mon, 6 Oct 2014 21:17:14 -0400 Subject: [PATCH] libnm: fix async property circular reference handling 3e5b3833 changed various libnm methods to return 0-length arrays rather than NULL, and changed various other places to assume this behavior. The guarantee that they didn't return NULL relied on the assumption that all D-Bus properties would get initialized by NMObject code before anyone could call their get methods. However, this assumption was violated in the case where two objects circularly referenced each other (eg, NMDevice and NMActiveConnection). f9f9d297 attempted to fix this by slightly changing the ordering of NMObjectCache operations, but it actually ended up breaking things and causing infinite loops of object creation in some cases. There's no way to guarantee we never return partially-initialized objects without heavily rewriting the object-creation code, so this reverts most of f9f9d297 (leaving only the new comment in _nm_object_create()). The crashes introduced by 3e5b3833 will no longer happen because we've now fixed the classes to have initialized their object arrays to non-NULL values even before they get the real values. --- libnm/nm-object.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/libnm/nm-object.c b/libnm/nm-object.c index 73dbade4a..0e82861fc 100644 --- a/libnm/nm-object.c +++ b/libnm/nm-object.c @@ -736,21 +736,7 @@ create_async_inited (GObject *object, GAsyncResult *result, gpointer user_data) NMObjectTypeAsyncData *async_data = user_data; GError *error = NULL; - if (g_async_initable_init_finish (G_ASYNC_INITABLE (object), result, &error)) { - NMObject *cached; - - /* Unlike in the sync case, in the async case we can't cache the object - * until it is completely initialized. But that means someone else might - * have been creating it at the same time, and they might have finished - * before us. - */ - cached = _nm_object_cache_get (nm_object_get_path (NM_OBJECT (object))); - if (cached) { - g_object_unref (object); - object = G_OBJECT (cached); - } else - _nm_object_cache_add (NM_OBJECT (object)); - } else { + if (!g_async_initable_init_finish (G_ASYNC_INITABLE (object), result, &error)) { dbgmsg ("Could not create object for %s: %s", nm_object_get_path (NM_OBJECT (object)), error->message); @@ -766,6 +752,17 @@ create_async_got_type (NMObjectTypeAsyncData *async_data, GType type) { GObject *object; + /* Ensure we don't have the object already; we may get multiple type + * requests for the same object if there are multiple properties on + * other objects that refer to the object at this path. One of those + * other requests may have already completed. + */ + object = (GObject *) _nm_object_cache_get (async_data->path); + if (object) { + create_async_complete (object, async_data); + return; + } + if (type == G_TYPE_INVALID) { /* Don't know how to create this object */ create_async_complete (NULL, async_data); @@ -776,6 +773,7 @@ create_async_got_type (NMObjectTypeAsyncData *async_data, GType type) NM_OBJECT_PATH, async_data->path, NM_OBJECT_DBUS_CONNECTION, async_data->connection, NULL); + _nm_object_cache_add (NM_OBJECT (object)); g_async_initable_init_async (G_ASYNC_INITABLE (object), G_PRIORITY_DEFAULT, NULL, create_async_inited, async_data); }