libnm: keep context-busy-watcher of NMClient alive for one more idle round

The context-busy-watch has two purposes:

1) it allows the user to watch whether the NMClient still has pending
  GSource'es attached to the GMainContext.
2) thereby, it also keeps the inner GMainContext integrated into the
  caller's (in case of synchronous initialization of NMClient).

Especially for 2), we must not get this wrong. Otherwise, we might
un-integrate the inner GMainContext too early and it will be leaked
indefinitely (because the user has no means to access or iterate it).

To be extra careful, extend the lifetime of the context-busy-watcher
for one more idle invocation. Theoretically, this should not be necessary,
but it's not clear whether something else is still pending.

The downside of that extra safety is that it is probably unnecessary in
practice. And in case where it is necessary, it hides an actual
issue, making it harder to notice and fix it.

1 files changed, 26 insertions, 0 deletions

diff --git a/libnm/nm-client.c b/libnm/nm-client.c
index b27890a831..89f4a02827 100644
--- a/libnm/nm-client.c
+++ b/libnm/nm-client.c
@@ -7617,6 +7617,32 @@ dispose (GObject *object)
 
 	nm_clear_pointer (&priv->udev, udev_unref);
 
+	if (   priv->context_busy_watcher
+	    && priv->dbus_context) {
+		GSource *cleanup_source;
+
+		/* Technically, we cancelled all pending actions (and these actions keep
+		 * the context_busy_watcher object alive). Also, we passed
+		 * no destroy notify to g_dbus_connection_signal_subscribe().
+		 * That means, there should be no other unaccounted GSource'es left.
+		 *
+		 * However, we really need to be sure that the context_busy_watcher's
+		 * lifetime matches the time that the context is busy. That is especially
+		 * important with synchronous initialization, where the context-busy-watcher
+		 * keeps the inner GMainContext integrated in the caller's.
+		 * We must not g_source_destroy() that integration too early.
+		 *
+		 * So to be really sure all this is given, always schedule one last
+		 * cleanup idle action with low priority. This should be the last
+		 * thing related to this instance that keeps the context busy. */
+		cleanup_source = nm_g_idle_source_new (G_PRIORITY_LOW + 10,
+		                                       nm_source_func_unref_gobject,
+		                                       g_steal_pointer (&priv->context_busy_watcher),
+		                                       NULL);
+		g_source_attach (cleanup_source, priv->dbus_context);
+		g_source_unref (cleanup_source);
+	}
+
 	nm_clear_pointer (&priv->dbus_context, g_main_context_unref);
 	nm_clear_pointer (&priv->main_context, g_main_context_unref);