From 2146c609963c695838126a0288dcd6f647af9abc Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Mon, 9 Nov 2015 18:35:37 +0100 Subject: [PATCH 1/3] libnm: stop using the private socket --- libnm/nm-dbus-helpers.c | 151 +--------------------------------------- 1 file changed, 3 insertions(+), 148 deletions(-) diff --git a/libnm/nm-dbus-helpers.c b/libnm/nm-dbus-helpers.c index 5d876e0c6a..a105f95c25 100644 --- a/libnm/nm-dbus-helpers.c +++ b/libnm/nm-dbus-helpers.c @@ -43,79 +43,9 @@ _nm_dbus_bus_type (void) return nm_bus; } -static struct { - GMutex mutex; - GWeakRef weak_ref; -} private_connection; - -static void -_private_dbus_connection_closed_cb (GDBusConnection *connection, - gboolean remote_peer_vanished, - GError *error, - gpointer user_data) -{ - GDBusConnection *p; - - g_mutex_lock (&private_connection.mutex); - p = g_weak_ref_get (&private_connection.weak_ref); - if (connection == p) { - g_signal_handlers_disconnect_by_func (G_OBJECT (connection), G_CALLBACK (_private_dbus_connection_closed_cb), NULL); - g_weak_ref_set (&private_connection.weak_ref, NULL); - } - if (p) - g_object_unref (p); - g_mutex_unlock (&private_connection.mutex); -} - -static GDBusConnection * -_private_dbus_connection_internalize (GDBusConnection *connection) -{ - GDBusConnection *p; - - g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), NULL); - g_return_val_if_fail (!g_dbus_connection_is_closed (connection), NULL); - - g_mutex_lock (&private_connection.mutex); - p = g_weak_ref_get (&private_connection.weak_ref); - if (p) { - g_object_unref (connection); - connection = p; - } else { - g_weak_ref_set (&private_connection.weak_ref, connection); - g_signal_connect (connection, "closed", G_CALLBACK (_private_dbus_connection_closed_cb), NULL); - } - g_mutex_unlock (&private_connection.mutex); - return connection; -} - GDBusConnection * _nm_dbus_new_connection (GCancellable *cancellable, GError **error) { - GDBusConnection *connection = NULL; - - /* If running as root try the private bus first */ - if (0 == geteuid () && !g_test_initialized ()) { - - GError *local = NULL; - GDBusConnection *p; - - p = g_weak_ref_get (&private_connection.weak_ref); - if (p) - return p; - - connection = g_dbus_connection_new_for_address_sync ("unix:path=" NMRUNDIR "/private", - G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, - NULL, cancellable, &local); - if (connection) - return _private_dbus_connection_internalize (connection); - - if (g_error_matches (local, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { - g_propagate_error (error, local); - return NULL; - } - g_error_free (local); - } - return g_bus_get_sync (_nm_dbus_bus_type (), cancellable, error); } @@ -136,65 +66,6 @@ new_connection_async_got_system (GObject *source, GAsyncResult *result, gpointer g_object_unref (simple); } -static void -new_connection_async_got_private (GObject *source, GAsyncResult *result, gpointer user_data) -{ - GSimpleAsyncResult *simple = user_data; - GDBusConnection *connection; - GError *error = NULL; - - connection = g_dbus_connection_new_for_address_finish (result, &error); - if (connection) { - connection = _private_dbus_connection_internalize (connection); - g_simple_async_result_set_op_res_gpointer (simple, connection, g_object_unref); - g_simple_async_result_complete (simple); - g_object_unref (simple); - return; - } - - if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { - g_simple_async_result_take_error (simple, error); - g_simple_async_result_complete (simple); - g_object_unref (simple); - return; - } - - g_clear_error (&error); - g_bus_get (_nm_dbus_bus_type (), - g_object_get_data (G_OBJECT (simple), "cancellable"), - new_connection_async_got_system, simple); -} - -static void -_nm_dbus_new_connection_async_do (GSimpleAsyncResult *simple, GCancellable *cancellable) -{ - g_dbus_connection_new_for_address ("unix:path=" NMRUNDIR "/private", - G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, - NULL, - cancellable, - new_connection_async_got_private, simple); -} - -static gboolean -_nm_dbus_new_connection_async_get_private (gpointer user_data) -{ - GSimpleAsyncResult *simple = user_data; - GDBusConnection *p; - - p = g_weak_ref_get (&private_connection.weak_ref); - if (!p) { - /* The connection is gone. Create a new one async... */ - _nm_dbus_new_connection_async_do (simple, - g_object_get_data (G_OBJECT (simple), "cancellable")); - } else { - g_simple_async_result_set_op_res_gpointer (simple, p, g_object_unref); - g_simple_async_result_complete (simple); - g_object_unref (simple); - } - - return G_SOURCE_REMOVE; -} - void _nm_dbus_new_connection_async (GCancellable *cancellable, GAsyncReadyCallback callback, @@ -204,25 +75,9 @@ _nm_dbus_new_connection_async (GCancellable *cancellable, simple = g_simple_async_result_new (NULL, callback, user_data, _nm_dbus_new_connection_async); - /* If running as root try the private bus first */ - if (0 == geteuid () && !g_test_initialized ()) { - GDBusConnection *p; - - if (cancellable) { - g_object_set_data_full (G_OBJECT (simple), "cancellable", - g_object_ref (cancellable), g_object_unref); - } - p = g_weak_ref_get (&private_connection.weak_ref); - if (p) { - g_object_unref (p); - g_idle_add (_nm_dbus_new_connection_async_get_private, simple); - } else - _nm_dbus_new_connection_async_do (simple, cancellable); - } else { - g_bus_get (_nm_dbus_bus_type (), - cancellable, - new_connection_async_got_system, simple); - } + g_bus_get (_nm_dbus_bus_type (), + cancellable, + new_connection_async_got_system, simple); } GDBusConnection * From 83b8b9e1f66ca325264cf0108a5a2025a7a9b77c Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Mon, 9 Nov 2015 18:28:36 +0100 Subject: [PATCH 2/3] bus-manager: drop private socket With ObjectManager we can not export ObjectSkeletons to multiple connections -- the manager would unexport the InterfaceSkeletons upon its destruction. It seems easiest to just drop the private socket altogether; It was broken for broken for some time and noone noticed anyway. Also startup before D-Bus is still broken: NetworkManager would reconnect to the bus but multiple managers won't notice the bus is around (we'll never see firewalld or policykit come up). We should probably just stop pretending we support operation without a real D-Bus server. With the advent of kdbus this makes even more sense. --- src/nm-bus-manager.c | 75 -------------------------------------------- 1 file changed, 75 deletions(-) diff --git a/src/nm-bus-manager.c b/src/nm-bus-manager.c index 1c09a7b68a..ed927959c0 100644 --- a/src/nm-bus-manager.c +++ b/src/nm-bus-manager.c @@ -35,9 +35,6 @@ #include "nm-exported-object.h" #include "NetworkManagerUtils.h" -#define PRIV_SOCK_PATH NMRUNDIR "/private" -#define PRIV_SOCK_TAG "private" - enum { DBUS_CONNECTION_CHANGED = 0, PRIVATE_CONNECTION_NEW, @@ -61,7 +58,6 @@ typedef struct { gboolean started; GSList *private_servers; - PrivateServer *priv_server; GDBusProxy *proxy; @@ -565,70 +561,12 @@ nm_bus_manager_get_unix_user (NMBusManager *self, /**************************************************************/ -static void -private_connection_new (NMBusManager *self, GDBusConnection *connection) -{ - NMBusManagerPrivate *priv = NM_BUS_MANAGER_GET_PRIVATE (self); - GHashTableIter iter; - NMExportedObject *object; - const char *path; - GError *error = NULL; - - /* Register all exported objects on this private connection */ - g_hash_table_iter_init (&iter, priv->exported); - while (g_hash_table_iter_next (&iter, (gpointer *) &path, (gpointer *) &object)) { - GSList *interfaces = nm_exported_object_get_interfaces (object); - - nm_assert_exported (self, path, object); - - for (; interfaces; interfaces = interfaces->next) { - GDBusInterfaceSkeleton *interface = G_DBUS_INTERFACE_SKELETON (interfaces->data); - - if (g_dbus_interface_skeleton_export (interface, connection, path, &error)) { - nm_log_trace (LOGD_CORE, "(%s) registered %p (%s) at '%s' on private socket.", - PRIV_SOCK_TAG, object, G_OBJECT_TYPE_NAME (interface), path); - } else { - nm_log_warn (LOGD_CORE, "(%s) could not register %p (%s) at '%s' on private socket: %s.", - PRIV_SOCK_TAG, object, G_OBJECT_TYPE_NAME (interface), path, - error->message); - g_clear_error (&error); - } - } - } -} - -static void -private_server_setup (NMBusManager *self) -{ - NMBusManagerPrivate *priv = NM_BUS_MANAGER_GET_PRIVATE (self); - - /* Skip this step if this is just a test program */ - if (nm_utils_get_testing ()) - return; - - /* Set up our main private DBus socket */ - if (mkdir (NMRUNDIR, 0755) == -1) { - if (errno != EEXIST) - nm_log_warn (LOGD_CORE, "Error creating directory \"%s\": %d (%s)", NMRUNDIR, errno, g_strerror (errno)); - } - priv->priv_server = private_server_new (PRIV_SOCK_PATH, PRIV_SOCK_TAG, self); - if (priv->priv_server) { - priv->private_servers = g_slist_append (priv->private_servers, priv->priv_server); - g_signal_connect (self, - NM_BUS_MANAGER_PRIVATE_CONNECTION_NEW "::" PRIV_SOCK_TAG, - (GCallback) private_connection_new, - NULL); - } -} - static void nm_bus_manager_init (NMBusManager *self) { NMBusManagerPrivate *priv = NM_BUS_MANAGER_GET_PRIVATE (self); priv->exported = g_hash_table_new (g_str_hash, g_str_equal); - - private_server_setup (self); } static void @@ -650,7 +588,6 @@ nm_bus_manager_dispose (GObject *object) g_slist_free_full (priv->private_servers, private_server_free); priv->private_servers = NULL; - priv->priv_server = NULL; nm_bus_manager_cleanup (self); @@ -879,8 +816,6 @@ nm_bus_manager_register_object (NMBusManager *self, NMExportedObject *object) { NMBusManagerPrivate *priv; - GDBusConnection *connection; - GHashTableIter iter; const char *path; GSList *interfaces, *ifs; @@ -915,16 +850,6 @@ nm_bus_manager_register_object (NMBusManager *self, priv->connection, path, NULL); } } - - if (priv->priv_server) { - g_hash_table_iter_init (&iter, priv->priv_server->connections); - while (g_hash_table_iter_next (&iter, (gpointer) &connection, NULL)) { - for (ifs = interfaces; ifs; ifs = ifs->next) { - g_dbus_interface_skeleton_export (G_DBUS_INTERFACE_SKELETON (ifs->data), - connection, path, NULL); - } - } - } } NMExportedObject * From b023d0754bdc094c91fea0ac9b73e017c49fc195 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 20 Jul 2015 17:30:02 -0500 Subject: [PATCH 3/3] exported-object: add support for DBus ObjectManager interface NMExportedObject now derives from GDBusObjectSkeleton, which is what GDBusObjectManagerServer wants. The main GDBusConnection and each private server connection now gets a new GDBusObjectManagerServer, and exported objects are registered with that instead of individually exporting each GDBusInterfaceSkeleton. Previously exported objects were not referenced by the BusManager, but instead removed from the exports hash via weak references. The GDBusObjectManagerServer instead references exported objects, which can make them live much longer than they did before. Co-Authored-By: Thomas Haller --- src/dhcp-manager/nm-dhcp-listener.c | 1 + src/nm-bus-manager.c | 323 +++++++++++------------- src/nm-bus-manager.h | 13 +- src/nm-exported-object.c | 42 ++- src/nm-exported-object.h | 4 +- src/nm-iface-helper.c | 4 +- src/nm-manager.c | 6 +- src/org.freedesktop.NetworkManager.conf | 2 + src/settings/nm-secret-agent.c | 2 +- 9 files changed, 194 insertions(+), 203 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-listener.c b/src/dhcp-manager/nm-dhcp-listener.c index 6c936e5bf3..25f09ff6c7 100644 --- a/src/dhcp-manager/nm-dhcp-listener.c +++ b/src/dhcp-manager/nm-dhcp-listener.c @@ -148,6 +148,7 @@ out: static void new_connection_cb (NMBusManager *mgr, GDBusConnection *connection, + GDBusObjectManager *manager, NMDhcpListener *self) { NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE (self); diff --git a/src/nm-bus-manager.c b/src/nm-bus-manager.c index ed927959c0..c3e86b2f8c 100644 --- a/src/nm-bus-manager.c +++ b/src/nm-bus-manager.c @@ -54,7 +54,7 @@ typedef struct _PrivateServer PrivateServer; typedef struct { GDBusConnection *connection; - GHashTable *exported; + GDBusObjectManagerServer *obj_manager; gboolean started; GSList *private_servers; @@ -69,6 +69,13 @@ static gboolean nm_bus_manager_init_bus (NMBusManager *self); static void nm_bus_manager_cleanup (NMBusManager *self); static void start_reconnection_timeout (NMBusManager *self); +/* The base path for our GDBusObjectManagerServers. They do not contain + * "NetworkManager" because GDBusObjectManagerServer requires that all + * exported objects be *below* the base path, and eg the Manager object + * is the base path already. + */ +#define OBJECT_MANAGER_SERVER_BASE_PATH "/org/freedesktop" + NM_DEFINE_SINGLETON_REGISTER (NMBusManager); NMBusManager * @@ -99,66 +106,23 @@ nm_bus_manager_setup (NMBusManager *instance) /**************************************************************/ -static void -nm_assert_exported (NMBusManager *self, const char *path, NMExportedObject *object) -{ -#if NM_MORE_ASSERTS - NMBusManagerPrivate *priv; - const char *p2, *po; - NMExportedObject *o2; - - /* NMBusManager and NMExportedObject are tied closely together. For example, while - * being registered, NMBusManager uses the path from nm_exported_object_get_path() - * as index. It relies on the path being stable. - * - * The alternative would be that NMBusManager copies the path upon registration - * to support diversion of NMExportedObject's path while being registered. But such - * a inconsistency would already indicate a bug, or at least a strange situation. - * - * So instead require some close cooperation between the two classes and add an - * assert here... */ - - nm_assert (NM_IS_BUS_MANAGER (self)); - nm_assert (!path || *path); - nm_assert (!object || NM_IS_EXPORTED_OBJECT (object)); - nm_assert (!!path || !!object); - - priv = NM_BUS_MANAGER_GET_PRIVATE (self); - nm_assert (priv->exported); - - if (!path) { - nm_assert (NM_IS_EXPORTED_OBJECT (object)); - - po = nm_exported_object_get_path (object); - nm_assert (po && *po); - - if (!g_hash_table_lookup_extended (priv->exported, po, (gpointer *) &p2, (gpointer *) &o2)) - nm_assert (FALSE); - - nm_assert (object == o2); - nm_assert (po == p2); - } else { - nm_assert (path && *path); - - if (!g_hash_table_lookup_extended (priv->exported, path, (gpointer *) &p2, (gpointer *) &o2)) - nm_assert (FALSE); - - nm_assert (NM_IS_EXPORTED_OBJECT (o2)); - nm_assert (!object || object == o2); - nm_assert (!g_strcmp0 (path, p2)); - nm_assert (p2 == nm_exported_object_get_path (o2)); - } -#endif -} - -/**************************************************************/ - struct _PrivateServer { const char *tag; GQuark detail; char *address; GDBusServer *server; - GHashTable *connections; + + /* With peer bus connections, we'll get a new connection for each + * client. For each connection we create an ObjectManager for + * that connection to handle exporting our objects. This table + * maps GDBusObjectManager :: 'fake sender'. + * + * Note that even for connections that don't export any objects + * we'll still create GDBusObjectManager since that's where we store + * the pointer to the GDBusConnection. + */ + GHashTable *obj_managers; + NMBusManager *manager; }; @@ -173,6 +137,8 @@ close_connection_in_idle (gpointer user_data) { CloseConnectionInfo *info = user_data; PrivateServer *server = info->server; + GHashTableIter iter; + GDBusObjectManagerServer *manager; /* Emit this for the manager */ g_signal_emit (server->manager, @@ -187,7 +153,14 @@ close_connection_in_idle (gpointer user_data) if (info->remote_peer_vanished) g_dbus_connection_close (info->connection, NULL, NULL, NULL); - g_hash_table_remove (server->connections, info->connection); + g_hash_table_iter_init (&iter, server->obj_managers); + while (g_hash_table_iter_next (&iter, (gpointer) &manager, NULL)) { + if (g_dbus_object_manager_server_get_connection (manager) == info->connection) { + g_hash_table_iter_remove (&iter); + break; + } + } + g_object_unref (server->manager); g_slice_free (CloseConnectionInfo, info); @@ -195,10 +168,10 @@ close_connection_in_idle (gpointer user_data) } static void -private_server_closed (GDBusConnection *conn, - gboolean remote_peer_vanished, - GError *error, - gpointer user_data) +private_server_closed_connection (GDBusConnection *conn, + gboolean remote_peer_vanished, + GError *error, + gpointer user_data) { PrivateServer *s = user_data; CloseConnectionInfo *info; @@ -226,13 +199,17 @@ private_server_new_connection (GDBusServer *server, { PrivateServer *s = user_data; static guint32 counter = 0; + GDBusObjectManagerServer *manager; char *sender; - g_signal_connect (conn, "closed", G_CALLBACK (private_server_closed), s); + g_signal_connect (conn, "closed", G_CALLBACK (private_server_closed_connection), s); /* Fake a sender since private connections don't have one */ sender = g_strdup_printf ("x:y:%d", counter++); - g_hash_table_insert (s->connections, g_object_ref (conn), sender); + + manager = g_dbus_object_manager_server_new (OBJECT_MANAGER_SERVER_BASE_PATH); + g_dbus_object_manager_server_set_connection (manager, conn); + g_hash_table_insert (s->obj_managers, manager, sender); nm_log_dbg (LOGD_CORE, "(%s) accepted connection %p on private socket.", s->tag, conn); @@ -241,16 +218,20 @@ private_server_new_connection (GDBusServer *server, g_signal_emit (s->manager, signals[PRIVATE_CONNECTION_NEW], s->detail, - conn); + conn, + manager); return TRUE; } static void -private_server_dbus_connection_destroy (GDBusConnection *conn) +private_server_manager_destroy (GDBusObjectManagerServer *manager) { - if (!g_dbus_connection_is_closed (conn)) - g_dbus_connection_close (conn, NULL, NULL, NULL); - g_object_unref (conn); + GDBusConnection *connection = g_dbus_object_manager_server_get_connection (manager); + + if (!g_dbus_connection_is_closed (connection)) + g_dbus_connection_close (connection, NULL, NULL, NULL); + g_dbus_object_manager_server_set_connection (manager, NULL); + g_object_unref (manager); } static gboolean @@ -304,9 +285,9 @@ private_server_new (const char *path, g_signal_connect (server, "new-connection", G_CALLBACK (private_server_new_connection), s); - s->connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, - (GDestroyNotify) private_server_dbus_connection_destroy, - g_free); + s->obj_managers = g_hash_table_new_full (g_direct_hash, g_direct_equal, + (GDestroyNotify) private_server_manager_destroy, + g_free); s->manager = manager; s->detail = g_quark_from_string (tag); s->tag = g_quark_to_string (s->detail); @@ -323,7 +304,7 @@ private_server_free (gpointer ptr) unlink (s->address); g_free (s->address); - g_hash_table_destroy (s->connections); + g_hash_table_destroy (s->obj_managers); g_dbus_server_stop (s->server); g_object_unref (s->server); @@ -360,10 +341,34 @@ nm_bus_manager_private_server_register (NMBusManager *self, static const char * private_server_get_connection_owner (PrivateServer *s, GDBusConnection *connection) { + GHashTableIter iter; + GDBusObjectManagerServer *manager; + const char *owner; + g_return_val_if_fail (s != NULL, NULL); g_return_val_if_fail (connection != NULL, NULL); - return g_hash_table_lookup (s->connections, connection); + g_hash_table_iter_init (&iter, s->obj_managers); + while (g_hash_table_iter_next (&iter, (gpointer) &manager, (gpointer) &owner)) { + if (g_dbus_object_manager_server_get_connection (manager) == connection) + return owner; + } + return NULL; +} + +static GDBusConnection * +private_server_get_connection_by_owner (PrivateServer *s, const char *owner) +{ + GHashTableIter iter; + GDBusObjectManagerServer *manager; + const char *priv_sender; + + g_hash_table_iter_init (&iter, s->obj_managers); + while (g_hash_table_iter_next (&iter, (gpointer) &manager, (gpointer) &priv_sender)) { + if (g_strcmp0 (owner, priv_sender) == 0) + return g_dbus_object_manager_server_get_connection (manager); + } + return NULL; } /**************************************************************/ @@ -451,7 +456,7 @@ _get_caller_info (NMBusManager *self, for (iter = priv->private_servers; iter; iter = g_slist_next (iter)) { PrivateServer *s = iter->data; - sender = g_hash_table_lookup (s->connections, connection); + sender = private_server_get_connection_owner (s, connection); if (sender) { if (out_uid) *out_uid = 0; @@ -534,17 +539,10 @@ nm_bus_manager_get_unix_user (NMBusManager *self, g_return_val_if_fail (out_uid != NULL, FALSE); /* Check if it's a private connection sender, which we fake */ - for (iter = priv->private_servers; iter; iter = g_slist_next (iter)) { - PrivateServer *s = iter->data; - GHashTableIter hiter; - const char *priv_sender; - - g_hash_table_iter_init (&hiter, s->connections); - while (g_hash_table_iter_next (&hiter, NULL, (gpointer) &priv_sender)) { - if (g_strcmp0 (sender, priv_sender) == 0) { - *out_uid = 0; - return TRUE; - } + for (iter = priv->private_servers; iter; iter = iter->next) { + if (private_server_get_connection_by_owner (iter->data, sender)) { + *out_uid = 0; + return TRUE; } } @@ -566,7 +564,7 @@ nm_bus_manager_init (NMBusManager *self) { NMBusManagerPrivate *priv = NM_BUS_MANAGER_GET_PRIVATE (self); - priv->exported = g_hash_table_new (g_str_hash, g_str_equal); + priv->obj_manager = g_dbus_object_manager_server_new (OBJECT_MANAGER_SERVER_BASE_PATH); } static void @@ -574,28 +572,31 @@ nm_bus_manager_dispose (GObject *object) { NMBusManager *self = NM_BUS_MANAGER (object); NMBusManagerPrivate *priv = NM_BUS_MANAGER_GET_PRIVATE (self); - - if (priv->exported) { - /* We don't take references to the registered objects. - * We rely on the objects to properly unregister. - * Especially, they must unregister before destroying the - * NMBusManager instance. */ - g_assert (g_hash_table_size (priv->exported) == 0); - - g_hash_table_destroy (priv->exported); - priv->exported = NULL; - } + GList *exported, *iter; g_slist_free_full (priv->private_servers, private_server_free); priv->private_servers = NULL; nm_bus_manager_cleanup (self); - if (priv->reconnect_id) { - g_source_remove (priv->reconnect_id); - priv->reconnect_id = 0; + if (priv->obj_manager) { + /* The ObjectManager owns the last reference to many exported + * objects, and when that reference is dropped the objects unregister + * themselves via nm_bus_manager_unregister_object(). By that time + * priv->obj_manager is already NULL and that prints warnings. Unregister + * them before clearing the ObjectManager instead. + */ + exported = g_dbus_object_manager_get_objects ((GDBusObjectManager *) priv->obj_manager); + for (iter = exported; iter; iter = iter->next) { + nm_bus_manager_unregister_object (self, iter->data); + g_object_unref (iter->data); + } + g_list_free (exported); + g_clear_object (&priv->obj_manager); } + nm_clear_g_source (&priv->reconnect_id); + G_OBJECT_CLASS (nm_bus_manager_parent_class)->dispose (object); } @@ -620,9 +621,8 @@ nm_bus_manager_class_init (NMBusManagerClass *klass) g_signal_new (NM_BUS_MANAGER_PRIVATE_CONNECTION_NEW, G_OBJECT_CLASS_TYPE (object_class), G_SIGNAL_RUN_LAST | G_SIGNAL_DETAILED, - G_STRUCT_OFFSET (NMBusManagerClass, private_connection_new), - NULL, NULL, NULL, - G_TYPE_NONE, 1, G_TYPE_POINTER); + 0, NULL, NULL, NULL, + G_TYPE_NONE, 2, G_TYPE_DBUS_CONNECTION, G_TYPE_DBUS_OBJECT_MANAGER_SERVER); signals[PRIVATE_CONNECTION_DISCONNECTED] = g_signal_new (NM_BUS_MANAGER_PRIVATE_CONNECTION_DISCONNECTED, @@ -648,6 +648,7 @@ nm_bus_manager_cleanup (NMBusManager *self) g_clear_object (&priv->connection); } + g_dbus_object_manager_server_set_connection (priv->obj_manager, NULL); priv->started = FALSE; } @@ -749,6 +750,7 @@ nm_bus_manager_init_bus (NMBusManager *self) return FALSE; } + g_dbus_object_manager_server_set_connection (priv->obj_manager, priv->connection); return TRUE; } @@ -813,119 +815,83 @@ nm_bus_manager_get_connection (NMBusManager *self) void nm_bus_manager_register_object (NMBusManager *self, - NMExportedObject *object) + GDBusObjectSkeleton *object) { NMBusManagerPrivate *priv; - const char *path; - GSList *interfaces, *ifs; g_return_if_fail (NM_IS_BUS_MANAGER (self)); g_return_if_fail (NM_IS_EXPORTED_OBJECT (object)); - path = nm_exported_object_get_path (object); - g_return_if_fail (path && *path); - priv = NM_BUS_MANAGER_GET_PRIVATE (self); - /* We hold a direct reference to the @path of the @object. Note that - * this requires the object not to modify the path as long as the object - * is registered. Especially, it must not free the path. - * - * This is a reasonable requirement, because having the object change - * the path while being registered is an awkward situation in the first - * place. While being registered, the @path and @interfaces must stay - * stable -- because the path is the identifier for the object in this - * situation. */ - - if (!nm_g_hash_table_insert (priv->exported, (gpointer) path, object)) +#if NM_MORE_ASSERTS >= 1 +#if GLIB_CHECK_VERSION(2,34,0) + if (g_dbus_object_manager_server_is_exported (priv->obj_manager, object)) g_return_if_reached (); +#endif +#endif - nm_assert_exported (self, path, object); - - interfaces = nm_exported_object_get_interfaces (object); - - if (priv->connection) { - for (ifs = interfaces; ifs; ifs = ifs->next) { - g_dbus_interface_skeleton_export (G_DBUS_INTERFACE_SKELETON (ifs->data), - priv->connection, path, NULL); - } - } + g_dbus_object_manager_server_export (priv->obj_manager, object); } -NMExportedObject * +GDBusObjectSkeleton * nm_bus_manager_get_registered_object (NMBusManager *self, const char *path) { - NMBusManagerPrivate *priv; - NMExportedObject *object; + NMBusManagerPrivate *priv = NM_BUS_MANAGER_GET_PRIVATE (self); - g_return_val_if_fail (NM_IS_BUS_MANAGER (self), NULL); - g_return_val_if_fail (path && *path, NULL); - - priv = NM_BUS_MANAGER_GET_PRIVATE (self); - - object = g_hash_table_lookup (priv->exported, path); - - if (object) - nm_assert_exported (self, path, object); - - return object; + return G_DBUS_OBJECT_SKELETON (g_dbus_object_manager_get_object ((GDBusObjectManager *) priv->obj_manager, path)); } void -nm_bus_manager_unregister_object (NMBusManager *self, NMExportedObject *object) +nm_bus_manager_unregister_object (NMBusManager *self, + GDBusObjectSkeleton *object) { NMBusManagerPrivate *priv; - GSList *interfaces; - const char *path; + gs_free char *path = NULL; g_return_if_fail (NM_IS_BUS_MANAGER (self)); g_return_if_fail (NM_IS_EXPORTED_OBJECT (object)); - path = nm_exported_object_get_path (object); - g_return_if_fail (path && *path); - - nm_assert_exported (self, NULL, object); - priv = NM_BUS_MANAGER_GET_PRIVATE (self); - if (!g_hash_table_remove (priv->exported, path)) +#if NM_MORE_ASSERTS >= 1 +#if GLIB_CHECK_VERSION(2,34,0) + if (!g_dbus_object_manager_server_is_exported (priv->obj_manager, object)) g_return_if_reached (); +#endif +#endif - for (interfaces = nm_exported_object_get_interfaces (object); interfaces; interfaces = interfaces->next) { - GDBusInterfaceSkeleton *interface = G_DBUS_INTERFACE_SKELETON (interfaces->data); + g_object_get (G_OBJECT (object), "g-object-path", &path, NULL); + g_return_if_fail (path != NULL); - if (g_dbus_interface_skeleton_get_object_path (interface)) - g_dbus_interface_skeleton_unexport (interface); - } + g_dbus_object_manager_server_unexport (priv->obj_manager, path); } -gboolean -nm_bus_manager_connection_is_private (NMBusManager *self, - GDBusConnection *connection) +const char * +nm_bus_manager_connection_get_private_name (NMBusManager *self, + GDBusConnection *connection) { NMBusManagerPrivate *priv; GSList *iter; + const char *owner; g_return_val_if_fail (NM_IS_BUS_MANAGER (self), FALSE); g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), FALSE); - if (g_dbus_connection_get_unique_name (connection)) - return FALSE; + if (g_dbus_connection_get_unique_name (connection)) { + /* Shortcut. The connection is not a private connection. */ + return NULL; + } - /* Assert that we still track the private connection. The caller - * of nm_bus_manager_connection_is_private() want's to subscribe - * to NM_BUS_MANAGER_PRIVATE_CONNECTION_DISCONNECTED, thus the signal - * never comes if we don't track the connection. */ priv = NM_BUS_MANAGER_GET_PRIVATE (self); for (iter = priv->private_servers; iter; iter = g_slist_next (iter)) { PrivateServer *s = iter->data; - if (g_hash_table_contains (s->connections, - connection)) - return TRUE; + if ((owner = private_server_get_connection_owner (s, connection))) + return owner; } - g_return_val_if_reached (TRUE); + g_return_val_if_reached (NULL); } /** @@ -952,8 +918,6 @@ nm_bus_manager_new_proxy (NMBusManager *self, const char *path, const char *iface) { - NMBusManagerPrivate *priv = NM_BUS_MANAGER_GET_PRIVATE (self); - GSList *iter; const char *owner; GDBusProxy *proxy; GError *error = NULL; @@ -962,15 +926,10 @@ nm_bus_manager_new_proxy (NMBusManager *self, g_return_val_if_fail (G_IS_DBUS_CONNECTION (connection), NULL); /* Might be a private connection, for which @name is fake */ - for (iter = priv->private_servers; iter; iter = g_slist_next (iter)) { - PrivateServer *s = iter->data; - - owner = private_server_get_connection_owner (s, connection); - if (owner) { - g_assert_cmpstr (owner, ==, name); - name = NULL; - break; - } + owner = nm_bus_manager_connection_get_private_name (self, connection); + if (owner) { + g_return_val_if_fail (!g_strcmp0 (owner, name), NULL); + name = NULL; } proxy = g_initable_new (proxy_type, NULL, &error, diff --git a/src/nm-bus-manager.h b/src/nm-bus-manager.h index 8dd4be7ba2..8e72a3ce99 100644 --- a/src/nm-bus-manager.h +++ b/src/nm-bus-manager.h @@ -72,8 +72,8 @@ gboolean nm_bus_manager_get_caller_info (NMBusManager *self, gulong *out_uid, gulong *out_pid); -gboolean nm_bus_manager_connection_is_private (NMBusManager *self, - GDBusConnection *connection); +const char *nm_bus_manager_connection_get_private_name (NMBusManager *self, + GDBusConnection *connection); gboolean nm_bus_manager_get_unix_user (NMBusManager *self, const char *sender, @@ -87,12 +87,13 @@ gboolean nm_bus_manager_get_caller_info_from_message (NMBusManager *self, gulong *out_pid); void nm_bus_manager_register_object (NMBusManager *self, - NMExportedObject *object); + GDBusObjectSkeleton *object); -void nm_bus_manager_unregister_object (NMBusManager *self, NMExportedObject *object); +void nm_bus_manager_unregister_object (NMBusManager *self, + GDBusObjectSkeleton *object); -NMExportedObject *nm_bus_manager_get_registered_object (NMBusManager *self, - const char *path); +GDBusObjectSkeleton *nm_bus_manager_get_registered_object (NMBusManager *self, + const char *path); void nm_bus_manager_private_server_register (NMBusManager *self, const char *path, diff --git a/src/nm-exported-object.c b/src/nm-exported-object.c index 0746fec0d4..ca3187e8ae 100644 --- a/src/nm-exported-object.c +++ b/src/nm-exported-object.c @@ -35,7 +35,7 @@ static gboolean quitting = FALSE; #define _ASSERT_NO_EARLY_EXPORT #endif -G_DEFINE_ABSTRACT_TYPE_WITH_CODE (NMExportedObject, nm_exported_object, G_TYPE_OBJECT, +G_DEFINE_ABSTRACT_TYPE_WITH_CODE (NMExportedObject, nm_exported_object, G_TYPE_DBUS_OBJECT_SKELETON, prefix_counters = g_hash_table_new (g_str_hash, g_str_equal); ) @@ -64,6 +64,19 @@ typedef struct { GQuark nm_exported_object_class_info_quark (void); G_DEFINE_QUARK (NMExportedObjectClassInfo, nm_exported_object_class_info) +/*****************************************************************************/ + +#define _NMLOG_PREFIX_NAME "exported-object" +#define _NMLOG_DOMAIN LOGD_CORE + +#define _NMLOG(level, ...) \ + nm_log (level, _NMLOG_DOMAIN, \ + "%s[%p]: " _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ + _NMLOG_PREFIX_NAME, (self) \ + _NM_UTILS_MACRO_REST (__VA_ARGS__)) + +/*****************************************************************************/ + /* "AddConnectionUnsaved" -> "handle-add-connection-unsaved" */ char * nm_exported_object_skeletonify_method_name (const char *dbus_method_name) @@ -461,6 +474,8 @@ nm_exported_object_create_skeletons (NMExportedObject *self, methods_len, (GObject *) self); + g_dbus_object_skeleton_add_interface ((GDBusObjectSkeleton *) self, interface); + priv->interfaces = g_slist_prepend (priv->interfaces, interface); } } @@ -498,6 +513,7 @@ nm_exported_object_destroy_skeletons (NMExportedObject *self) GDBusInterfaceSkeleton *interface = priv->interfaces->data; priv->interfaces = g_slist_delete_link (priv->interfaces, priv->interfaces); + g_dbus_object_skeleton_remove_interface ((GDBusObjectSkeleton *) self, interface); nm_exported_object_skeleton_release (interface); } } @@ -533,6 +549,11 @@ nm_exported_object_export (NMExportedObject *self) nm_assert (priv->_constructed); #endif + priv->bus_mgr = nm_bus_manager_get (); + if (!priv->bus_mgr) + g_return_val_if_reached (NULL); + g_object_add_weak_pointer ((GObject *) priv->bus_mgr, (gpointer *) &priv->bus_mgr); + class_export_path = NM_EXPORTED_OBJECT_GET_CLASS (self)->export_path; p = strchr (class_export_path, '%'); if (p) { @@ -551,18 +572,19 @@ nm_exported_object_export (NMExportedObject *self) } else priv->path = g_strdup (class_export_path); + _LOGT ("export: \"%s\"", priv->path); + g_dbus_object_skeleton_set_object_path (G_DBUS_OBJECT_SKELETON (self), priv->path); + type = G_OBJECT_TYPE (self); while (type != NM_TYPE_EXPORTED_OBJECT) { nm_exported_object_create_skeletons (self, type); type = g_type_parent (type); } - priv->bus_mgr = g_object_ref (nm_bus_manager_get ()); - /* Important: priv->path and priv->interfaces must not change while * the object is registered. */ - nm_bus_manager_register_object (priv->bus_mgr, self); + nm_bus_manager_register_object (priv->bus_mgr, (GDBusObjectSkeleton *) self); return priv->path; } @@ -615,17 +637,23 @@ nm_exported_object_unexport (NMExportedObject *self) priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self); g_return_if_fail (priv->path); - g_return_if_fail (priv->bus_mgr); /* Important: priv->path and priv->interfaces must not change while * the object is registered. */ - nm_bus_manager_unregister_object (priv->bus_mgr, self); + _LOGT ("unexport: \"%s\"", priv->path); + + if (priv->bus_mgr) { + nm_bus_manager_unregister_object (priv->bus_mgr, (GDBusObjectSkeleton *) self); + g_object_remove_weak_pointer ((GObject *) priv->bus_mgr, (gpointer *) &priv->bus_mgr); + priv->bus_mgr = NULL; + } nm_exported_object_destroy_skeletons (self); + g_dbus_object_skeleton_set_object_path ((GDBusObjectSkeleton *) self, NULL); + g_clear_pointer (&priv->path, g_free); - g_clear_object (&priv->bus_mgr); if (nm_clear_g_source (&priv->notify_idle_id)) { /* We had a notification queued. Since we removed all interfaces, diff --git a/src/nm-exported-object.h b/src/nm-exported-object.h index 3f69a7bda8..88efb6fe98 100644 --- a/src/nm-exported-object.h +++ b/src/nm-exported-object.h @@ -52,11 +52,11 @@ void nm_exported_object_skeleton_release (GDBusInterfaceSkeleton *interface); #define NM_EXPORTED_OBJECT_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_EXPORTED_OBJECT, NMExportedObjectClass)) struct _NMExportedObject { - GObject parent; + GDBusObjectSkeleton parent; }; typedef struct { - GObjectClass parent; + GDBusObjectSkeletonClass parent; const char *export_path; char export_on_construction; diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index fdd714f918..396929ff75 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -523,7 +523,7 @@ gconstpointer nm_config_get (void); const char *nm_config_get_dhcp_client (gpointer unused); gboolean nm_config_get_configure_and_quit (gpointer unused); gconstpointer nm_bus_manager_get (void); -void nm_bus_manager_register_object (gpointer unused, const char *path, gpointer object); +void nm_bus_manager_register_object (gpointer unused, gpointer object); void nm_bus_manager_unregister_object (gpointer unused, gpointer object); gconstpointer @@ -551,7 +551,7 @@ nm_bus_manager_get (void) } void -nm_bus_manager_register_object (gpointer unused, const char *path, gpointer object) +nm_bus_manager_register_object (gpointer unused, gpointer object) { } diff --git a/src/nm-manager.c b/src/nm-manager.c index 5cba730fec..bb92a13f38 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4461,7 +4461,7 @@ prop_set_auth_done_cb (NMAuthChain *chain, NMAuthCallResult result; GDBusMessage *reply = NULL; const char *error_message; - NMExportedObject *object; + gs_unref_object NMExportedObject *object = NULL; const NMGlobalDnsConfig *global_dns; gs_unref_variant GVariant *value = NULL; GVariant *args; @@ -4477,8 +4477,8 @@ prop_set_auth_done_cb (NMAuthChain *chain, goto done; } - object = nm_bus_manager_get_registered_object (priv->dbus_mgr, - g_dbus_message_get_path (pfd->message)); + object = NM_EXPORTED_OBJECT (nm_bus_manager_get_registered_object (priv->dbus_mgr, + g_dbus_message_get_path (pfd->message))); if (!object) { reply = g_dbus_message_new_method_error (pfd->message, "org.freedesktop.DBus.Error.UnknownObject", diff --git a/src/org.freedesktop.NetworkManager.conf b/src/org.freedesktop.NetworkManager.conf index e6f030643d..c4082d2b50 100644 --- a/src/org.freedesktop.NetworkManager.conf +++ b/src/org.freedesktop.NetworkManager.conf @@ -37,6 +37,8 @@ send_interface="org.freedesktop.DBus.Introspectable"/> + bus_mgr = g_object_ref (nm_bus_manager_get ()); priv->connection = g_object_ref (connection); - priv->connection_is_private = nm_bus_manager_connection_is_private (priv->bus_mgr, connection); + priv->connection_is_private = !!nm_bus_manager_connection_get_private_name (priv->bus_mgr, connection); _LOGt ("constructed: %s, owner=%s%s%s (%s), private-connection=%d, unique-name=%s%s%s", (description = _create_description (dbus_owner, identifier, uid)),