keep-alive: default keep-alive instance to alive

NMKeepAlive supports conditions (watches, bindings) that determine
whether the instance signals alive state. Currently, there are two
conditions: watch a VISIBLE state of a connection and watch whether a
D-Bus client is registered.

Anyway, if none of these conditions are enabled, then the default should
be that the instance is alive. If one or more conditions are enabled,
then they cooprate in a particular way, depending on whether the
condition is enabled and whether it is satisfied.

Previously, the code couldn't differenciate whether a D-Bus watch was
not enabled or whether the D-Bus client already disconnected. It would
just check whether priv->dbus_client was set, but that alone was not
sufficient to differentiate. That means, we couldn't determine whether
the keep-alive instance is alive (by default, as no D-Bus watch is
enabled) or whether the keep-alive instance is dead (because the D-Bus
client disconnected). Fix that, by tracking explicitly whether to watch
based on a priv->dbus_client_watching flag.

Note, that problems by this were avoided earlier, by only arming the
instance when enabling a condition. But I think the concept of arming the
keep-alive instance merely means that the instance is ready to fire
events. It should not mean that the user only arms the instance after
registering a condition.

Also, without this, we couldn't unbind the NMKeepAlive instance from all
conditions, without making it dead right away. Unbinding/disabling all
conditions should render the instance alive, not dead.

1 files changed, 31 insertions, 13 deletions

diff --git a/src/nm-keep-alive.c b/src/nm-keep-alive.c
index 052113bf5b..cb495baac3 100644
--- a/src/nm-keep-alive.c
+++ b/src/nm-keep-alive.c
@@ -48,6 +48,7 @@ typedef struct {
 
 	bool alive:1;
 	bool dbus_client_confirmed:1;
+	bool dbus_client_watching:1;
 } NMKeepAlivePrivate;
 
 struct _NMKeepAlive {
@@ -87,18 +88,31 @@ _is_alive (NMKeepAlive *self)
 		return TRUE;
 	}
 
-	if (   priv->connection
-	    && NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection),
-	                     NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE))
-		return TRUE;
+	if (priv->dbus_client_watching) {
+		if (_is_alive_dbus_client (self)) {
+			/* no matter what, the keep-alive is alive, because there is a D-Bus client
+			 * still around keeping it alive. */
+			return TRUE;
+		}
+		/* the D-Bus client is gone. The only other binding (below) for the connection's
+		 * visibility cannot keep the instance alive.
+		 *
+		 * As such, a D-Bus client watch is authorative and overrules other conditions (that
+		 * we have so far). */
+		return FALSE;
+	}
 
-	/* Perform this check as last. We want to confirm whether the dbus-client
-	 * is alive lazyly, so if we already decided above that the keep-alive
-	 * is good, we don't rely on the outcome of this check. */
-	if (_is_alive_dbus_client (self))
-		return TRUE;
+	if (priv->connection) {
+		if (!NM_FLAGS_HAS (nm_settings_connection_get_flags (priv->connection),
+		                   NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) {
+			/* the connection is invisible (and apparently has no D-Bus client
+			 * watch above). The instance is dead. */
+			return FALSE;
+		}
+	}
 
-	return FALSE;
+	/* by default, the instance is alive. */
+	return TRUE;
 }
 
 static void
@@ -287,13 +301,16 @@ nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self,
 {
 	NMKeepAlivePrivate *priv = NM_KEEP_ALIVE_GET_PRIVATE (self);
 
+	if (priv->disarmed)
+		return;
+
 	cleanup_dbus_watch (self);
 
-	if (   client_address
-	    && !priv->disarmed) {
+	if (client_address) {
 		_LOGD ("Registering dbus client watch for keep alive");
 
 		priv->dbus_client = g_strdup (client_address);
+		priv->dbus_client_watching = TRUE;
 		priv->dbus_client_confirmed = FALSE;
 		priv->dbus_connection = g_object_ref (connection);
 		priv->subscription_id = g_dbus_connection_signal_subscribe (connection,
@@ -306,7 +323,8 @@ nm_keep_alive_set_dbus_client_watch (NMKeepAlive *self,
 		                                                            name_owner_changed_cb,
 		                                                            self,
 		                                                            NULL);
-	}
+	} else
+		priv->dbus_client_watching = FALSE;
 
 	_notify_alive (self);
 }