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.
This commit is contained in:
Thomas Haller 2017-06-15 19:14:21 +02:00
parent c5a48b7a0b
commit 529d620a59

View file

@ -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