modem: cleanup nm_modem_deactivate_async()th/no-gasyncresult

- fix stopping ppp-manager. Previously, we would take a reference
  to priv->ppp_manager to cancel it later. However, deactivate_cleanup()
  is called first, which already issues nm_ppp_manager_stop().
  Thereby, not using a callback and not waiting for the operation
  to complete.

- get rid of this "step" state machine. There are litterally two steps
  that need to be performed asynchornously. Instead chain the calls.

- it is now obviously visible, that the async callback never completes
  synchronously upon being called (provided that all async operations
  that it calls themself have this behavior -- which they should).

4 files changed, 69 insertions, 96 deletions

diff --git a/src/devices/wwan/nm-modem-broadband.c b/src/devices/wwan/nm-modem-broadband.c
index 12d3c1e8e0..824fd6db42 100644
--- a/src/devices/wwan/nm-modem-broadband.c
+++ b/src/devices/wwan/nm-modem-broadband.c
@@ -1188,9 +1188,11 @@ disconnect (NMModem *modem,
 /*****************************************************************************/
 
 static void
-deactivate_cleanup (NMModem *_self, NMDevice *device)
+deactivate_cleanup (NMModem *modem,
+                    NMDevice *device,
+                    gboolean stop_ppp_manager)
 {
-	NMModemBroadband *self = NM_MODEM_BROADBAND (_self);
+	NMModemBroadband *self = NM_MODEM_BROADBAND (modem);
 
 	/* TODO: cancel SimpleConnect() if any */
 
@@ -1201,8 +1203,9 @@ deactivate_cleanup (NMModem *_self, NMDevice *device)
 
 	self->_priv.pin_tries = 0;
 
-	/* Chain up parent's */
-	NM_MODEM_CLASS (nm_modem_broadband_parent_class)->deactivate_cleanup (_self, device);
+	NM_MODEM_CLASS (nm_modem_broadband_parent_class)->deactivate_cleanup (modem,
+	                                                                      device,
+	                                                                      stop_ppp_manager);
 }
 
 /*****************************************************************************/
diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c
index eb93c25745..56b6d745c5 100644
--- a/src/devices/wwan/nm-modem-ofono.c
+++ b/src/devices/wwan/nm-modem-ofono.c
@@ -252,7 +252,9 @@ disconnect (NMModem *modem,
 }
 
 static void
-deactivate_cleanup (NMModem *modem, NMDevice *device)
+deactivate_cleanup (NMModem *modem,
+                    NMDevice *device,
+                    gboolean stop_ppp_manager)
 {
 	NMModemOfono *self = NM_MODEM_OFONO (modem);
 	NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self);
@@ -261,7 +263,9 @@ deactivate_cleanup (NMModem *modem, NMDevice *device)
 
 	g_clear_object (&priv->ip4_config);
 
-	NM_MODEM_CLASS (nm_modem_ofono_parent_class)->deactivate_cleanup (modem, device);
+	NM_MODEM_CLASS (nm_modem_ofono_parent_class)->deactivate_cleanup (modem,
+	                                                                  device,
+	                                                                  stop_ppp_manager);
 }
 
 static gboolean
diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c
index 3d8dee83dd..56d9811645 100644
--- a/src/devices/wwan/nm-modem.c
+++ b/src/devices/wwan/nm-modem.c
@@ -1105,7 +1105,9 @@ nm_modem_complete_connection (NMModem *self,
 /*****************************************************************************/
 
 static void
-deactivate_cleanup (NMModem *self, NMDevice *device)
+deactivate_cleanup (NMModem *self,
+                    NMDevice *device,
+                    gboolean stop_ppp_manager)
 {
 	NMModemPrivate *priv;
 	int ifindex;
@@ -1126,7 +1128,8 @@ deactivate_cleanup (NMModem *self, NMDevice *device)
 
 	if (priv->ppp_manager) {
 		g_signal_handlers_disconnect_by_data (priv->ppp_manager, self);
-		nm_ppp_manager_stop (priv->ppp_manager, NULL, NULL, NULL);
+		if (stop_ppp_manager)
+			nm_ppp_manager_stop (priv->ppp_manager, NULL, NULL, NULL);
 		g_clear_object (&priv->ppp_manager);
 	}
 
@@ -1157,121 +1160,69 @@ deactivate_cleanup (NMModem *self, NMDevice *device)
 
 /*****************************************************************************/
 
-typedef enum {
-	DEACTIVATE_CONTEXT_STEP_FIRST,
-	DEACTIVATE_CONTEXT_STEP_CLEANUP,
-	DEACTIVATE_CONTEXT_STEP_PPP_MANAGER_STOP,
-	DEACTIVATE_CONTEXT_STEP_MM_DISCONNECT,
-	DEACTIVATE_CONTEXT_STEP_LAST
-} DeactivateContextStep;
-
 typedef struct {
 	NMModem *self;
 	NMDevice *device;
 	GCancellable *cancellable;
 	NMModemDeactivateCallback callback;
 	gpointer callback_user_data;
-	DeactivateContextStep step;
-	NMPPPManager *ppp_manager;
 } DeactivateContext;
 
 static void
 deactivate_context_complete (DeactivateContext *ctx, GError *error)
 {
+	NMModem *self = ctx->self;
+
+	_LOGD ("modem deactivation finished %s%s%s",
+	       NM_PRINT_FMT_QUOTED (error, "with failure: ", error->message, "", "successfully"));
+
 	if (ctx->callback)
 		ctx->callback (ctx->self, error, ctx->callback_user_data);
-	nm_g_object_unref (ctx->ppp_manager);
 	nm_g_object_unref (ctx->cancellable);
 	g_object_unref (ctx->device);
 	g_object_unref (ctx->self);
 	g_slice_free (DeactivateContext, ctx);
 }
 
-static void deactivate_step (DeactivateContext *ctx);
-
 static void
-disconnect_ready (NMModem *self,
-                  GError *error,
-                  gpointer user_data)
+_deactivate_call_disconnect_cb (NMModem *self,
+                                GError *error,
+                                gpointer user_data)
 {
-	DeactivateContext *ctx = user_data;
-
-	if (error) {
-		deactivate_context_complete (ctx, error);
-		return;
-	}
-
-	ctx->step++;
-	deactivate_step (ctx);
+	deactivate_context_complete (user_data, error);
 }
 
 static void
-ppp_manager_stop_ready (NMPPPManager *ppp_manager,
-                        NMPPPManagerStopHandle *handle,
-                        gboolean was_cancelled,
-                        gpointer user_data)
+_deactivate_call_disconnect (DeactivateContext *ctx)
 {
-	DeactivateContext *ctx = user_data;
-
-	if (was_cancelled)
-		return;
-
-	ctx->step++;
-	deactivate_step (ctx);
+	NM_MODEM_GET_CLASS (ctx->self)->disconnect (ctx->self,
+	                                            FALSE,
+	                                            ctx->cancellable,
+	                                            _deactivate_call_disconnect_cb,
+	                                            ctx);
 }
 
 static void
-deactivate_step (DeactivateContext *ctx)
+_deactivate_ppp_manager_stop_cb (NMPPPManager *ppp_manager,
+                                 NMPPPManagerStopHandle *handle,
+                                 gboolean was_cancelled,
+                                 gpointer user_data)
 {
-	NMModem *self = ctx->self;
-	NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self);
-	GError *error = NULL;
+	DeactivateContext *ctx = user_data;
 
-	/* Check cancellable in each step */
-	if (g_cancellable_set_error_if_cancelled (ctx->cancellable, &error)) {
-		deactivate_context_complete (ctx, error);
-		g_error_free (error);
-		return;
-	}
+	g_object_unref (ppp_manager);
 
-	switch (ctx->step) {
-	case DEACTIVATE_CONTEXT_STEP_FIRST:
-		ctx->step++;
-		/* fall through */
-	case DEACTIVATE_CONTEXT_STEP_CLEANUP:
-		/* Make sure we keep a ref to the PPP manager if there is one */
-		if (priv->ppp_manager)
-			ctx->ppp_manager = g_object_ref (priv->ppp_manager);
-		/* Run cleanup */
-		NM_MODEM_GET_CLASS (self)->deactivate_cleanup (self, ctx->device);
-		ctx->step++;
-		/* fall through */
-	case DEACTIVATE_CONTEXT_STEP_PPP_MANAGER_STOP:
-		/* If we have a PPP manager, stop it */
-		if (ctx->ppp_manager) {
-			nm_ppp_manager_stop (ctx->ppp_manager,
-			                     ctx->cancellable,
-			                     ppp_manager_stop_ready,
-			                     ctx);
-		}
-		ctx->step++;
-		/* fall through */
-	case DEACTIVATE_CONTEXT_STEP_MM_DISCONNECT:
-		/* Disconnect asynchronously */
-		NM_MODEM_GET_CLASS (self)->disconnect (self,
-		                                       FALSE,
-		                                       ctx->cancellable,
-		                                       disconnect_ready,
-		                                       ctx);
-		return;
+	if (was_cancelled) {
+		gs_free_error GError *error = NULL;
 
-	case DEACTIVATE_CONTEXT_STEP_LAST:
-		_LOGD ("modem deactivation finished");
-		deactivate_context_complete (ctx, NULL);
+		if (!g_cancellable_set_error_if_cancelled (ctx->cancellable, &error))
+			nm_assert_not_reached ();
+		deactivate_context_complete (ctx, error);
 		return;
 	}
 
-	g_assert_not_reached ();
+	nm_assert (!g_cancellable_is_cancelled (ctx->cancellable));
+	_deactivate_call_disconnect (ctx);
 }
 
 void
@@ -1281,24 +1232,37 @@ nm_modem_deactivate_async (NMModem *self,
                            NMModemDeactivateCallback callback,
                            gpointer user_data)
 {
+	NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self);
 	DeactivateContext *ctx;
+	NMPPPManager *ppp_manager;
 
 	g_return_if_fail (NM_IS_MODEM (self));
 	g_return_if_fail (NM_IS_DEVICE (device));
 	g_return_if_fail (G_IS_CANCELLABLE (cancellable));
 
-	ctx = g_slice_new0 (DeactivateContext);
+	ctx = g_slice_new (DeactivateContext);
 	ctx->self = g_object_ref (self);
 	ctx->device = g_object_ref (device);
 	ctx->cancellable = g_object_ref (cancellable);
 	ctx->callback = callback;
 	ctx->callback_user_data = user_data;
 
-	/* Start */
-	ctx->step = DEACTIVATE_CONTEXT_STEP_FIRST;
-	deactivate_step (ctx);
+	ppp_manager = nm_g_object_ref (priv->ppp_manager);
+
+	NM_MODEM_GET_CLASS (self)->deactivate_cleanup (self, ctx->device, FALSE);
+
+	if (ppp_manager) {
+		/* If we have a PPP manager, stop it.
+		 *
+		 * Pass on the reference in @ppp_manager. */
+		nm_ppp_manager_stop (ppp_manager,
+		                     ctx->cancellable,
+		                     _deactivate_ppp_manager_stop_cb,
+		                     ctx);
+		return;
+	}
 
-	/* FIXME: never invoke the callback syncronously. */
+	_deactivate_call_disconnect (ctx);
 }
 
 /*****************************************************************************/
@@ -1307,7 +1271,7 @@ void
 nm_modem_deactivate (NMModem *self, NMDevice *device)
 {
 	/* First cleanup */
-	NM_MODEM_GET_CLASS (self)->deactivate_cleanup (self, device);
+	NM_MODEM_GET_CLASS (self)->deactivate_cleanup (self, device, TRUE);
 	/* Then disconnect without waiting */
 	NM_MODEM_GET_CLASS (self)->disconnect (self, FALSE, NULL, NULL, NULL);
 }
@@ -1346,7 +1310,7 @@ nm_modem_device_state_changed (NMModem *self,
 			if (new_state == NM_DEVICE_STATE_FAILED || new_state == NM_DEVICE_STATE_DISCONNECTED)
 				warn = FALSE;
 			/* First cleanup */
-			NM_MODEM_GET_CLASS (self)->deactivate_cleanup (self, NULL);
+			NM_MODEM_GET_CLASS (self)->deactivate_cleanup (self, NULL, TRUE);
 			NM_MODEM_GET_CLASS (self)->disconnect (self, warn, NULL, NULL, NULL);
 		}
 		break;
diff --git a/src/devices/wwan/nm-modem.h b/src/devices/wwan/nm-modem.h
index e51ceab830..f7b6bfe959 100644
--- a/src/devices/wwan/nm-modem.h
+++ b/src/devices/wwan/nm-modem.h
@@ -156,7 +156,9 @@ typedef struct {
 	                                            _NMModemDisconnectCallback callback,
 	                                            gpointer user_data);
 
-	void     (*deactivate_cleanup)             (NMModem *self, NMDevice *device);
+	void     (*deactivate_cleanup)             (NMModem *self,
+	                                            NMDevice *device,
+	                                            gboolean stop_ppp_manager);
 
 	gboolean (*owns_port)                      (NMModem *self, const char *iface);
 } NMModemClass;