core: better ignore deactivations before a new activation starts (rh #1058843)

When a new activation request comes in and the device is already
activated, two NMActRequests will exist for the device in parallel.
The old one handles de-activation of the device and is then disposed,
while the new one waits until the device is de-activated and then
takes over and starts the new activation.

Both requests are watching device state, and the new request may
mis-interpret the de-activation states and clean up its device pointer,
leading to assertion failures when the new activation starts.

To fix this (and because NMVPNConnection *does* always want to see
de-activation events from the device) remove the code that tries to
ignore de-activation from NMActiveConnection's device state handler.

Instead, have NMActRequest skip any reaction to device state changes
unless it is the current activation request on the device.  The VPN
code always wants to see the device's state, so it doesn't need this
check.

2 files changed, 9 insertions, 19 deletions

diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c
index 5fa0fc3b9f..7ffa3dc6f5 100644
--- a/src/nm-activation-request.c
+++ b/src/nm-activation-request.c
@@ -307,6 +307,10 @@ device_state_changed (NMActiveConnection *active,
 {
 	NMActiveConnectionState ac_state = NM_ACTIVE_CONNECTION_STATE_UNKNOWN;
 
+	/* Ignore state changes when this activation request is not yet active */
+	if (NM_ACTIVE_CONNECTION (nm_device_get_act_request (device)) != active)
+		return;
+
 	/* Set NMActiveConnection state based on the device's state */
 	switch (new_state) {
 	case NM_DEVICE_STATE_PREPARE:
diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c
index e6fe7cd01b..e8d07dd23a 100644
--- a/src/nm-active-connection.c
+++ b/src/nm-active-connection.c
@@ -325,28 +325,12 @@ device_state_changed (NMDevice *device,
 	NMActiveConnection *self = NM_ACTIVE_CONNECTION (user_data);
 	NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self);
 
+	/* When already deactivated or before activation, device state changes are useless */
+	if (priv->state >= NM_ACTIVE_CONNECTION_STATE_DEACTIVATED)
+		return;
 	if (old_state < NM_DEVICE_STATE_DISCONNECTED)
 		return;
 
-	if (old_state > NM_DEVICE_STATE_DISCONNECTED) {
-		/* Ignore disconnects if this ActiveConnection has not yet started
-		 * activating.  This is caused by activating a device when it's
-		 * already activated, which causes a deactivating of the device before
-		 * activating the new connection.
-		 */
-		if (new_state == NM_DEVICE_STATE_DISCONNECTED &&
-		    old_state > NM_DEVICE_STATE_DISCONNECTED &&
-		    priv->state == NM_ACTIVE_CONNECTION_STATE_UNKNOWN) {
-			return;
-		}
-
-		/* If the device used to be active, but now is disconnected/failed, we
-		 * no longer care about its state.
-		 */
-		if (new_state <= NM_DEVICE_STATE_DISCONNECTED || new_state == NM_DEVICE_STATE_FAILED)
-			g_signal_handlers_disconnect_by_func (device, G_CALLBACK (device_state_changed), self);
-	}
-
 	/* Let subclasses handle the state change */
 	if (NM_ACTIVE_CONNECTION_GET_CLASS (self)->device_state_changed)
 		NM_ACTIVE_CONNECTION_GET_CLASS (self)->device_state_changed (self, device, new_state, old_state);
@@ -362,6 +346,8 @@ device_master_changed (GObject *object,
 	NMActiveConnection *master;
 	NMActiveConnectionState master_state;
 
+	if (NM_ACTIVE_CONNECTION (nm_device_get_act_request (device)) != self)
+		return;
 	if (!nm_device_get_master (device))
 		return;
 	g_signal_handlers_disconnect_by_func (device, G_CALLBACK (device_master_changed), self);