exported-object: fix source interface for PropertiesChanged D-Bus signal

nm_exported_object_notify() hooks GObject's property-change signal
and searches for the D-Bus interface to which to send the
PropertiesChanged signal.
Then it would enqueue the value encoded as GVariant in pending_notifications.
However, thereby the association between the property that changed and the
interface was lost. So later in idle_emit_properties_changed() it would
just pick the first interface with a properties-changed-id.

That is wrong. pending_notifications must be associated with the D-Bus
interface that we are going to notify. That is, each InterfaceData must
have its own separate list.

This is broken since introducing NMExportedObject and moving to gdbus.
Only now it was discovered as NMDevice itself has two D-Bus interfaces:
"Device" and "Device.Statistics".

Note that the order of the PropertiesChanged in our D-Bus API is not defined
so that later signals can reach the receiver before earlier signals.
Also, multiple change signals for one property may be combined.
That is not changed by this patch and is not considered a bug, but something
that our D-Bus API wrt. PropertiesChanged does not guarantee.

https://bugzilla.gnome.org/show_bug.cgi?id=770629

src/nm-exported-object.c

@@ -38,14 +38,13 @@ G_DEFINE_ABSTRACT_TYPE (NMExportedObject, nm_exported_object, G_TYPE_DBUS_OBJECT
 typedef struct {
 	GDBusInterfaceSkeleton *interface;
 	guint property_changed_signal_id;
+	GHashTable *pending_notifies;
 } InterfaceData;
 
 typedef struct {
 	NMBusManager *bus_mgr;
 	char *path;
 
-	GHashTable *pending_notifies;
-
 	InterfaceData *interfaces;
 	guint num_interfaces;
 
@@ -268,7 +267,7 @@ nm_exported_object_class_add_interface (NMExportedObjectClass *object_class,
 		                  nm_exported_object_class_info_quark (), classinfo);
 	}
 
-	classinfo->skeleton_types = g_slist_prepend (classinfo->skeleton_types,
+	classinfo->skeleton_types = g_slist_append (classinfo->skeleton_types,
 	                                            GSIZE_TO_POINTER (dbus_skeleton_type));
 
 	/* Ensure @dbus_skeleton_type's class_init has run, so its signals/properties
@@ -492,6 +491,11 @@ nm_exported_object_create_skeletons (NMExportedObject *self,
 		g_dbus_object_skeleton_add_interface ((GDBusObjectSkeleton *) self, ifdata->interface);
 
 		ifdata->property_changed_signal_id = g_signal_lookup ("properties-changed", G_OBJECT_TYPE (ifdata->interface));
+
+		ifdata->pending_notifies = g_hash_table_new_full (g_direct_hash,
+		                                                  g_direct_equal,
+		                                                  NULL,
+		                                                  (GDestroyNotify) g_variant_unref);
 	}
 	nm_assert (i == 0);
 
@@ -542,6 +546,7 @@ nm_exported_object_destroy_skeletons (NMExportedObject *self)
 
 		g_dbus_object_skeleton_remove_interface ((GDBusObjectSkeleton *) self, ifdata->interface);
 		nm_exported_object_skeleton_release (ifdata->interface);
+		g_hash_table_destroy (ifdata->pending_notifies);
 	}
 
 	g_slice_free1 (sizeof (InterfaceData) * n, priv->interfaces);
@@ -701,11 +706,7 @@ nm_exported_object_unexport (NMExportedObject *self)
 
 	g_clear_pointer (&priv->path, g_free);
 
-	if (nm_clear_g_source (&priv->notify_idle_id)) {
+	nm_clear_g_source (&priv->notify_idle_id);
-		/* We had a notification queued. Since we removed all interfaces,
-		 * the notification is obsolete and must be cleaned up. */
-		g_hash_table_remove_all (priv->pending_notifies);
-	}
 }
 
 /*****************************************************************************/
@@ -784,23 +785,33 @@ static gboolean
 idle_emit_properties_changed (gpointer self)
 {
 	NMExportedObjectPrivate *priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self);
-	gs_unref_variant GVariant *variant = NULL;
+	guint k;
-	InterfaceData *ifdata = NULL;
-	GHashTableIter hash_iter;
-	GVariantBuilder notifies;
-	guint i, n;
-	PendingNotifiesItem *values;
 
 	priv->notify_idle_id = 0;
 
+	for (k = 0; k < priv->num_interfaces; k++) {
+		InterfaceData *ifdata = &priv->interfaces[k];
+		gs_unref_variant GVariant *variant = NULL;
+		PendingNotifiesItem *values;
+		GVariantBuilder notifies;
+		GHashTableIter hash_iter;
+		guint i, n;
 
-	n = g_hash_table_size (priv->pending_notifies);
+		n = g_hash_table_size (ifdata->pending_notifies);
-	g_return_val_if_fail (n > 0, FALSE);
+		if (n == 0)
+			continue;
 
+		if (!ifdata->property_changed_signal_id)
+			goto next;
+
+		/* We use here alloca in a loop, something that is usually avoided.
+		 * But the number of interfaces "priv->num_interfaces" is small (determined by
+		 * the depth of the type inheritence) and the number of possible pending_notifies
+		 * "n" is small (determined by the number of GObject properties). */
 		values = g_alloca (sizeof (values[0]) * n);
 
 		i = 0;
-	g_hash_table_iter_init (&hash_iter, priv->pending_notifies);
+		g_hash_table_iter_init (&hash_iter, ifdata->pending_notifies);
 		while (g_hash_table_iter_next (&hash_iter, (gpointer) &values[i].property_name, (gpointer) &values[i].variant))
 			i++;
 		nm_assert (i == n);
@@ -812,25 +823,22 @@ idle_emit_properties_changed (gpointer self)
 			g_variant_builder_add (&notifies, "{sv}", values[i].property_name, values[i].variant);
 		variant = g_variant_ref_sink (g_variant_builder_end (&notifies));
 
-	g_hash_table_remove_all (priv->pending_notifies);
-
-	for (i = 0; i < priv->num_interfaces; i++) {
-		if (priv->interfaces[i].property_changed_signal_id != 0) {
-			ifdata = &priv->interfaces[i];
-			break;
-		}
-	}
-	g_return_val_if_fail (ifdata, FALSE);
 
 		if (nm_logging_enabled (LOGL_DEBUG, LOGD_DBUS_PROPS)) {
 			gs_free char *notification = g_variant_print (variant, TRUE);
 
-		nm_log_dbg (LOGD_DBUS_PROPS, "PropertiesChanged %s %p: %s",
+			nm_log_dbg (LOGD_DBUS_PROPS, "PropertiesChanged %s, %s, %p: %s",
-		            G_OBJECT_TYPE_NAME (self), self, notification);
+			            G_OBJECT_TYPE_NAME (self), G_OBJECT_TYPE_NAME (ifdata->interface),
+			            self, notification);
 		}
 
 		g_signal_emit (ifdata->interface, ifdata->property_changed_signal_id, 0, variant);
-	return FALSE;
+
+next:
+		g_hash_table_remove_all (ifdata->pending_notifies);
+	}
+
+	return G_SOURCE_REMOVE;
 }
 
 static void
@@ -841,6 +849,7 @@ nm_exported_object_notify (GObject *object, GParamSpec *pspec)
 	GType type;
 	const char *dbus_property_name = NULL;
 	GValue value = G_VALUE_INIT;
+	InterfaceData *ifdata = NULL;
 	const GVariantType *vtype;
 	guint i, j;
 
@@ -863,10 +872,10 @@ nm_exported_object_notify (GObject *object, GParamSpec *pspec)
 	}
 
 	for (i = 0; i < priv->num_interfaces; i++) {
-		GDBusInterfaceSkeleton *skel = priv->interfaces[i].interface;
 		GDBusInterfaceInfo *iinfo;
 
-		iinfo = g_dbus_interface_skeleton_get_info (skel);
+		ifdata = &priv->interfaces[i];
+		iinfo = g_dbus_interface_skeleton_get_info (ifdata->interface);
 		for (j = 0; iinfo->properties[j]; j++) {
 			if (nm_streq (iinfo->properties[j]->name, dbus_property_name)) {
 				vtype = G_VARIANT_TYPE (iinfo->properties[j]->signature);
@@ -882,7 +891,7 @@ vtype_found:
 
 	/* @dbus_property_name is inside classinfo and never freed, thus we don't clone it.
 	 * Also, we do a pointer, not string comparison. */
-	g_hash_table_insert (priv->pending_notifies,
+	g_hash_table_insert (ifdata->pending_notifies,
 	                     (gpointer) dbus_property_name,
 	                     g_dbus_gvalue_to_gvariant (&value, vtype));
 	g_value_unset (&value);
@@ -896,12 +905,6 @@ vtype_found:
 static void
 nm_exported_object_init (NMExportedObject *self)
 {
-	NMExportedObjectPrivate *priv = NM_EXPORTED_OBJECT_GET_PRIVATE (self);
-
-	priv->pending_notifies = g_hash_table_new_full (g_direct_hash,
-	                                                g_direct_equal,
-	                                                NULL,
-	                                                (GDestroyNotify) g_variant_unref);
 }
 
 static void
@@ -937,7 +940,6 @@ nm_exported_object_dispose (GObject *object)
 	} else
 		g_clear_pointer (&priv->path, g_free);
 
-	g_clear_pointer (&priv->pending_notifies, g_hash_table_destroy);
 	nm_clear_g_source (&priv->notify_idle_id);
 
 	G_OBJECT_CLASS (nm_exported_object_parent_class)->dispose (object);