core: convert NMDBusObject's "path" property to signal "exported-changed"

The GObject property "path" exists for the sole reasons so that
other components can connect to the "notify::path" signal.

However, notifications are blocked by g_object_freeze_notify(),
and especially for NMDBusObject we want to make use of that to
combine multiple PropertiesChanged events into one.

This blocking of the signal is not desired for the case where
we wait for "notify::path". Convert that to a signal, which
will not be blocked.

5 files changed, 77 insertions, 38 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 86c4b3373a..651390db60 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -9577,9 +9577,8 @@ nm_device_activate_ip6_state_done (NMDevice *self)
 /*****************************************************************************/
 
 static void
-act_request_set_cb (NMActRequest *act_request,
-                    GParamSpec *pspec,
-                    NMDevice *self)
+act_request_exported_changed (NMActRequest *act_request,
+                              NMDevice *self)
 {
 	_notify (self, PROP_ACTIVE_CONNECTION);
 }
@@ -9610,8 +9609,8 @@ act_request_set (NMDevice *self, NMActRequest *act_request)
 
 	if (act_request) {
 		priv->act_request_id = g_signal_connect (act_request,
-		                                         "notify::"NM_DBUS_OBJECT_PATH,
-		                                         G_CALLBACK (act_request_set_cb),
+		                                         NM_DBUS_OBJECT_EXPORTED_CHANGED,
+		                                         G_CALLBACK (act_request_exported_changed),
 		                                         self);
 
 		switch (nm_active_connection_get_activation_type (NM_ACTIVE_CONNECTION (act_request))) {
@@ -15220,7 +15219,7 @@ get_property (GObject *object, guint prop_id,
 		                      g_variant_new ("(uu)", priv->state, priv->state_reason));
 		break;
 	case PROP_ACTIVE_CONNECTION:
-		nm_dbus_utils_g_value_set_object_path (value, priv->act_request_public ? priv->act_request : NULL);
+		nm_dbus_utils_g_value_set_object_path_still_exported (value, priv->act_request_public ? priv->act_request : NULL);
 		break;
 	case PROP_DEVICE_TYPE:
 		g_value_set_uint (value, priv->type);
diff --git a/src/nm-dbus-object.c b/src/nm-dbus-object.c
index cd3a23509e..014f30f40c 100644
--- a/src/nm-dbus-object.c
+++ b/src/nm-dbus-object.c
@@ -37,9 +37,13 @@ nm_dbus_object_set_quitting (void)
 
 /*****************************************************************************/
 
-NM_GOBJECT_PROPERTIES_DEFINE (NMDBusObject,
-	PROP_PATH,
-);
+enum {
+	EXPORTED_CHANGED,
+
+	LAST_SIGNAL,
+};
+
+static guint signals[LAST_SIGNAL] = { 0 };
 
 G_DEFINE_ABSTRACT_TYPE (NMDBusObject, nm_dbus_object, G_TYPE_OBJECT);
 
@@ -53,6 +57,12 @@ G_DEFINE_ABSTRACT_TYPE (NMDBusObject, nm_dbus_object, G_TYPE_OBJECT);
 
 /*****************************************************************************/
 
+static void
+_emit_exported_changed (NMDBusObject *self)
+{
+	g_signal_emit (self, signals[EXPORTED_CHANGED], 0);
+}
+
 static char *
 _create_export_path (NMDBusObjectClass *klass)
 {
@@ -107,6 +117,8 @@ nm_dbus_object_export (NMDBusObject *self)
 
 	g_return_val_if_fail (!self->internal.path, self->internal.path);
 
+	nm_assert (!self->internal.is_unexporting);
+
 	self->internal.path = _create_export_path (NM_DBUS_OBJECT_GET_CLASS (self));
 
 	self->internal.export_version_id = ++id_counter;
@@ -115,7 +127,7 @@ nm_dbus_object_export (NMDBusObject *self)
 
 	_nm_dbus_manager_obj_export (self);
 
-	_notify (self, PROP_PATH);
+	_emit_exported_changed (self);
 	return self->internal.path;
 }
 
@@ -135,12 +147,27 @@ nm_dbus_object_unexport (NMDBusObject *self)
 
 	_LOGT ("unexport: \"%s\"", self->internal.path);
 
+	/* note that we emit the signal *before* actually unexporting the object.
+	 * The reason is, that listeners want to use this signal to know that
+	 * the object goes away, and clear their D-Bus path to this object.
+	 *
+	 * But this must happen before we actually unregister the object, so
+	 * that we first emit a D-Bus signal that other objects no longer
+	 * reference this object, before finally unregistering the object itself.
+	 *
+	 * The inconvenient part is, that at this point nm_dbus_object_get_path()
+	 * still returns the path. So, the callee needs to handle that. Possibly
+	 * by using "nm_dbus_object_get_path_still_exported()". */
+	self->internal.is_unexporting = TRUE;
+
+	_emit_exported_changed (self);
+
 	_nm_dbus_manager_obj_unexport (self);
 
 	g_clear_pointer (&self->internal.path, g_free);
 	self->internal.export_version_id = 0;
 
-	_notify (self, PROP_PATH);
+	self->internal.is_unexporting = FALSE;
 }
 
 /*****************************************************************************/
@@ -205,22 +232,6 @@ nm_dbus_object_emit_signal (NMDBusObject *self,
 /*****************************************************************************/
 
 static void
-get_property (GObject *object, guint prop_id,
-              GValue *value, GParamSpec *pspec)
-{
-	NMDBusObject *self = NM_DBUS_OBJECT (object);
-
-	switch (prop_id) {
-	case PROP_PATH:
-		g_value_set_string (value, self->internal.path);
-		break;
-	default:
-		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
-		break;
-	}
-}
-
-static void
 dispatch_properties_changed (GObject *object,
                              guint n_pspecs,
                              GParamSpec **pspecs)
@@ -284,10 +295,11 @@ dispose (GObject *object)
 			g_warn_if_reached ();
 			nm_dbus_object_unexport (self);
 		}
-	} else if (nm_clear_g_free (&self->internal.path)) {
+	} else if (self->internal.path) {
 		/* FIXME: do a proper, coordinate shutdown, so that no objects stay
 		 * alive nor exported. */
-		_notify (self, PROP_PATH);
+		_emit_exported_changed (self);
+		nm_clear_g_free (&self->internal.path);
 	}
 
 	G_OBJECT_CLASS (nm_dbus_object_parent_class)->dispose (object);
@@ -302,14 +314,13 @@ nm_dbus_object_class_init (NMDBusObjectClass *klass)
 
 	object_class->constructed = constructed;
 	object_class->dispose = dispose;
-	object_class->get_property = get_property;
 	object_class->dispatch_properties_changed = dispatch_properties_changed;
 
-	obj_properties[PROP_PATH] =
-	    g_param_spec_string (NM_DBUS_OBJECT_PATH, "", "",
-	                         NULL,
-	                         G_PARAM_READABLE |
-	                         G_PARAM_STATIC_STRINGS);
-
-	g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties);
+	signals[EXPORTED_CHANGED] =
+	    g_signal_new (NM_DBUS_OBJECT_EXPORTED_CHANGED,
+	                  G_OBJECT_CLASS_TYPE (object_class),
+	                  G_SIGNAL_RUN_FIRST,
+	                  0, NULL, NULL,
+	                  g_cclosure_marshal_VOID__VOID,
+	                  G_TYPE_NONE, 0);
 }
diff --git a/src/nm-dbus-object.h b/src/nm-dbus-object.h
index f6629289a5..947ea6c7dd 100644
--- a/src/nm-dbus-object.h
+++ b/src/nm-dbus-object.h
@@ -82,7 +82,7 @@ extern const NMDBusInterfaceInfoExtended nm_interface_info_device_statistics;
 #define NM_IS_DBUS_OBJECT_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass),  NM_TYPE_DBUS_OBJECT))
 #define NM_DBUS_OBJECT_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj),  NM_TYPE_DBUS_OBJECT, NMDBusObjectClass))
 
-#define NM_DBUS_OBJECT_PATH "path"
+#define NM_DBUS_OBJECT_EXPORTED_CHANGED "exported-changed"
 
 /* NMDBusObject and NMDBusManager cooperate strongly. Hence, there is an
  * internal data structure attached to the NMDBusObject accessible to both of them. */
@@ -99,6 +99,7 @@ struct _NMDBusObjectInternal {
 	 * unexported, or even re-exported afterwards. If that happens, we want
 	 * to fail the request. For that, we keep track of a version id.  */
 	guint64 export_version_id;
+	bool is_unexporting:1;
 };
 
 struct _NMDBusObject {
@@ -169,6 +170,18 @@ nm_dbus_object_is_exported (NMDBusObject *self)
 	return !!nm_dbus_object_get_path (self);
 }
 
+static inline const char *
+nm_dbus_object_get_path_still_exported (NMDBusObject *self)
+{
+	g_return_val_if_fail (NM_IS_DBUS_OBJECT (self), NULL);
+
+	/* like nm_dbus_object_get_path(), however, while unexporting
+	 * (exported-changed signal), returns %NULL instead of the path. */
+	return self->internal.is_unexporting
+	       ? NULL
+	       : self->internal.path;
+}
+
 const char *nm_dbus_object_export      (NMDBusObject *self);
 void        nm_dbus_object_unexport    (NMDBusObject *self);
 
diff --git a/src/nm-dbus-utils.c b/src/nm-dbus-utils.c
index 7f7fb27873..b79cc34371 100644
--- a/src/nm-dbus-utils.c
+++ b/src/nm-dbus-utils.c
@@ -121,6 +121,20 @@ nm_dbus_utils_g_value_set_object_path (GValue *value, gpointer object)
 }
 
 void
+nm_dbus_utils_g_value_set_object_path_still_exported (GValue *value, gpointer object)
+{
+	const char *path;
+
+	g_return_if_fail (!object || NM_IS_DBUS_OBJECT (object));
+
+	if (   object
+	    && (path = nm_dbus_object_get_path_still_exported (object)))
+		g_value_set_string (value, path);
+	else
+		g_value_set_string (value, "/");
+}
+
+void
 nm_dbus_utils_g_value_set_object_path_from_hash (GValue *value,
                                                  GHashTable *hash /* has keys of NMDBusObject type. */,
                                                  gboolean expect_all_exported)
diff --git a/src/nm-dbus-utils.h b/src/nm-dbus-utils.h
index 62945cf14e..3107a29c82 100644
--- a/src/nm-dbus-utils.h
+++ b/src/nm-dbus-utils.h
@@ -173,6 +173,8 @@ const char **nm_dbus_utils_get_paths_for_clist (const struct CList *lst_head,
 
 void nm_dbus_utils_g_value_set_object_path (GValue *value, gpointer object);
 
+void nm_dbus_utils_g_value_set_object_path_still_exported (GValue *value, gpointer object);
+
 void nm_dbus_utils_g_value_set_object_path_from_hash (GValue *value,
                                                       GHashTable *hash,
                                                       gboolean expect_all_exported);