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.

1 files changed, 13 insertions, 15 deletions

diff --git a/libnm/nm-object.c b/libnm/nm-object.c
index 73dbade4ab..0e82861fc4 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);
 }