From 529d620a59f42e3f06bd4e7de5c817f175803c0d Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 15 Jun 2017 19:14:21 +0200
Subject: [PATCH] libnm: don't use async constructor for GDBusObjectManager

The current implementation of GDBusObjectManagerClient implements
GAsyncInitableIface, however it simply runs GInitableIface's
synchronous init on another thread.

I suspect that there are races in the way that is implemented.
For one, we see crashes and warnings (rh#1450075, rh#1457769,
rh#1457223). Also, it seems very wrong to me, how GDBusObjectManagerClient
mixes asynchronous signals (on_control_proxy_g_signal) with
synchronously getting all objects (process_get_all_result,
GetManagedObjects).

I think we should ditch GDBusObjectManager altogether, including the
gdbus-codegen skeletons. They add layers of code, for something that
should be simple to do directly. For now, just don't do asynchronous
initialization on another thread, so we at least avoid this kind of
multithreadding issue.

This may make the initialization of NMClient a bit slower.
---
 libnm/nm-client.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/libnm/nm-client.c b/libnm/nm-client.c
index 1349127718..774f5f6ba0 100644
--- a/libnm/nm-client.c
+++ b/libnm/nm-client.c
@@ -2321,6 +2321,7 @@ typedef struct {
 	GCancellable *cancellable;
 	GSimpleAsyncResult *result;
 	int pending_init;
+	guint idle_init_id;
 } NMClientInitData;
 
 static void
@@ -2329,6 +2330,7 @@ init_async_complete (NMClientInitData *init_data)
 	g_simple_async_result_complete (init_data->result);
 	g_object_unref (init_data->result);
 	g_clear_object (&init_data->cancellable);
+	nm_clear_g_source (&init_data->idle_init_id);
 	g_slice_free (NMClientInitData, init_data);
 }
 
@@ -2411,8 +2413,8 @@ new_object_manager (GObject *source_object, GAsyncResult *res, gpointer user_dat
 	g_clear_object (&priv->new_object_manager_cancellable);
 }
 
-static void
-got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data)
+static gboolean
+got_object_manager (gpointer user_data)
 {
 	NMClientInitData *init_data = user_data;
 	NMClient *client;
@@ -2422,11 +2424,26 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data)
 	GError *error = NULL;
 	GDBusObjectManager *object_manager;
 
-	object_manager = g_dbus_object_manager_client_new_for_bus_finish (result, &error);
+	init_data->idle_init_id = 0;
+
+	if (g_cancellable_set_error_if_cancelled (init_data->cancellable,
+	                                          &error)) {
+		g_simple_async_result_take_error (init_data->result, error);
+		init_async_complete (init_data);
+		return G_SOURCE_REMOVE;
+	}
+
+	object_manager = g_dbus_object_manager_client_new_for_bus_sync (_nm_dbus_bus_type (),
+	                                                                G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START,
+	                                                                "org.freedesktop.NetworkManager",
+	                                                                "/org/freedesktop",
+	                                                                proxy_type, NULL, NULL,
+	                                                                init_data->cancellable,
+	                                                                &error);
 	if (object_manager == NULL) {
 		g_simple_async_result_take_error (init_data->result, error);
 		init_async_complete (init_data);
-		return;
+		return G_SOURCE_REMOVE;
 	}
 
 	client = init_data->client;
@@ -2439,7 +2456,7 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data)
 		if (!objects_created (client, priv->object_manager, &error)) {
 			g_simple_async_result_take_error (init_data->result, error);
 			init_async_complete (init_data);
-			return;
+			return G_SOURCE_REMOVE;
 		}
 
 		objects = g_dbus_object_manager_get_objects (priv->object_manager);
@@ -2462,6 +2479,7 @@ got_object_manager (GObject *object, GAsyncResult *result, gpointer user_data)
 
 	g_signal_connect (priv->object_manager, "notify::name-owner",
 	                  G_CALLBACK (name_owner_changed), client);
+	return G_SOURCE_REMOVE;
 }
 
 static void
@@ -2479,14 +2497,7 @@ prepare_object_manager (NMClient *client,
 	                                               user_data, init_async);
 	g_simple_async_result_set_op_res_gboolean (init_data->result, TRUE);
 
-	g_dbus_object_manager_client_new_for_bus (_nm_dbus_bus_type (),
-	                                          G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_DO_NOT_AUTO_START,
-	                                          "org.freedesktop.NetworkManager",
-	                                          "/org/freedesktop",
-	                                          proxy_type, NULL, NULL,
-	                                          init_data->cancellable,
-	                                          got_object_manager,
-	                                          init_data);
+	init_data->idle_init_id = g_idle_add (got_object_manager, init_data);
 }
 
 static void