iface-modem-3gpp: allow cancelling the wait for packet service state

When a request to disable the modem arrives while the packet service
state wait is ongoing we were not correctly cancelling the operation.
The main reason for this is that this operation does not change the
modem state, and so the "wait for final state" logic in the disabling
sequence was not being considered.

We solve this by plugging in the Simple.Connect() operation
cancellable in the wait for packet service state operation. The
connection attempt will be cancelled during the disabling sequence as
well, and when that happens we will explicitly halt the packet service
state wait as well.

3 files changed, 88 insertions, 21 deletions

diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index c9e109e9c..f6f8c8812 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -135,8 +135,9 @@ GET_NETWORK_SUPPORTED (5gs, 5GS)
 
 typedef struct {
     MMModem3gppPacketServiceState final_state;
-    gulong state_changed_id;
-    guint timeout_id;
+    gulong                        state_changed_id;
+    guint                         timeout_id;
+    gulong                        cancellable_id;
 } WaitForPacketServiceStateContext;
 
 MMModem3gppPacketServiceState
@@ -166,29 +167,61 @@ wait_for_packet_service_state_context_complete (GTask                         *t
     self = g_task_get_source_object (task);
     ctx = g_task_get_task_data (task);
 
-    if (ctx->state_changed_id) {
-        if (g_signal_handler_is_connected (self, ctx->state_changed_id))
-            g_signal_handler_disconnect (self, ctx->state_changed_id);
-        ctx->state_changed_id = 0;
-    }
+    g_assert (ctx->state_changed_id);
+    if (g_signal_handler_is_connected (self, ctx->state_changed_id))
+        g_signal_handler_disconnect (self, ctx->state_changed_id);
+    ctx->state_changed_id = 0;
+
+    g_assert (ctx->timeout_id);
+    g_source_remove (ctx->timeout_id);
+    ctx->timeout_id = 0;
 
-    if (ctx->timeout_id) {
-        g_source_remove (ctx->timeout_id);
-        ctx->timeout_id = 0;
+    if (!g_task_return_error_if_cancelled (task)) {
+        if (ctx->cancellable_id) {
+            g_cancellable_disconnect (g_task_get_cancellable (task), ctx->cancellable_id);
+            ctx->cancellable_id = 0;
+        }
+        if (error)
+            g_task_return_error (task, error);
+        else
+            g_task_return_int (task, state);
     }
+    g_object_unref (task);
+}
 
-    if (error)
-        g_task_return_error (task, error);
-    else
-        g_task_return_int (task, state);
+static void
+packet_service_wait_cancelled (GCancellable *cancellable,
+                               GTask        *task)
+{
+    MMIfaceModem3gpp                 *self;
+    WaitForPacketServiceStateContext *ctx;
 
-    g_object_unref (task);
+    self = g_task_get_source_object (task);
+    ctx = g_task_get_task_data (task);
+
+    mm_obj_dbg (self, "wait for packet service state '%s': cancelled",
+                (ctx->final_state == MM_MODEM_3GPP_PACKET_SERVICE_STATE_UNKNOWN) ?
+                "any" : mm_modem_3gpp_packet_service_state_get_string (ctx->final_state));
+
+    /* Given that the cancellable is the same one as in the task, we can complete the operation here
+     * without specifying an exact error. The task will itself be completed with a cancelled error. */
+    g_assert (g_task_get_cancellable (task) == cancellable);
+    wait_for_packet_service_state_context_complete (task, MM_MODEM_3GPP_PACKET_SERVICE_STATE_UNKNOWN, NULL);
 }
 
 static gboolean
-packet_service_state_changed_wait_expired (GTask *task)
+packet_service_wait_timeout (GTask *task)
 {
-    GError *error;
+    MMIfaceModem3gpp                 *self;
+    WaitForPacketServiceStateContext *ctx;
+    GError                           *error;
+
+    self = g_task_get_source_object (task);
+    ctx = g_task_get_task_data (task);
+
+    mm_obj_dbg (self, "wait for packet service state '%s': timed out",
+                (ctx->final_state == MM_MODEM_3GPP_PACKET_SERVICE_STATE_UNKNOWN) ?
+                "any" : mm_modem_3gpp_packet_service_state_get_string (ctx->final_state));
 
     error = g_error_new (MM_CORE_ERROR,
                          MM_CORE_ERROR_RETRY,
@@ -218,6 +251,10 @@ packet_service_state_changed (MMIfaceModem3gpp *self,
         (state != ctx->final_state))
         return;
 
+    mm_obj_dbg (self, "wait for packet service state '%s': finished",
+                (ctx->final_state == MM_MODEM_3GPP_PACKET_SERVICE_STATE_UNKNOWN) ?
+                "any" : mm_modem_3gpp_packet_service_state_get_string (ctx->final_state));
+
     /* Done! */
     wait_for_packet_service_state_context_complete (task, state, NULL);
 }
@@ -225,6 +262,7 @@ packet_service_state_changed (MMIfaceModem3gpp *self,
 void
 mm_iface_modem_3gpp_wait_for_packet_service_state (MMIfaceModem3gpp              *self,
                                                    MMModem3gppPacketServiceState  final_state,
+                                                   GCancellable                  *cancellable,
                                                    GAsyncReadyCallback            callback,
                                                    gpointer                       user_data)
 {
@@ -232,7 +270,7 @@ mm_iface_modem_3gpp_wait_for_packet_service_state (MMIfaceModem3gpp
     WaitForPacketServiceStateContext *ctx;
     GTask                            *task;
 
-    task = g_task_new (self, NULL, callback, user_data);
+    task = g_task_new (self, cancellable, callback, user_data);
 
     g_object_get (self,
                   MM_IFACE_MODEM_3GPP_PACKET_SERVICE_STATE, &state,
@@ -252,6 +290,10 @@ mm_iface_modem_3gpp_wait_for_packet_service_state (MMIfaceModem3gpp
 
     g_task_set_task_data (task, ctx, g_free);
 
+    /* Ownership of the task will be shared among the signal handler, the timeout,
+     * and the cancellable. As soon as one of them is triggered, it should cancel the
+     * other two. */
+
     /* Want to get notified when packet service state changes */
     ctx->state_changed_id = g_signal_connect (self,
                                               "notify::" MM_IFACE_MODEM_3GPP_PACKET_SERVICE_STATE,
@@ -259,8 +301,23 @@ mm_iface_modem_3gpp_wait_for_packet_service_state (MMIfaceModem3gpp
                                               task);
     /* But we don't want to wait forever */
     ctx->timeout_id = g_timeout_add_seconds (10,
-                                             (GSourceFunc)packet_service_state_changed_wait_expired,
+                                             (GSourceFunc)packet_service_wait_timeout,
                                              task);
+
+    /* And we want it to be cancellable */
+    if (cancellable) {
+        ctx->cancellable_id = g_cancellable_connect (cancellable,
+                                                     (GCallback) packet_service_wait_cancelled,
+                                                     task,
+                                                     NULL);
+        /* Do nothing if already cancelled, packet_service_wait_cancelled() will already be called */
+        if (!ctx->cancellable_id)
+            return;
+    }
+
+    mm_obj_dbg (self, "wait for packet service state '%s': started",
+                (final_state == MM_MODEM_3GPP_PACKET_SERVICE_STATE_UNKNOWN) ?
+                "any" : mm_modem_3gpp_packet_service_state_get_string (final_state));
 }
 
 /*****************************************************************************/
diff --git a/src/mm-iface-modem-3gpp.h b/src/mm-iface-modem-3gpp.h
index 5054570f4..424e33ebd 100644
--- a/src/mm-iface-modem-3gpp.h
+++ b/src/mm-iface-modem-3gpp.h
@@ -407,6 +407,7 @@ gboolean mm_iface_modem_3gpp_set_packet_service_state_finish (MMIfaceModem3gpp
 /* Allow waiting for packet service */
 void                          mm_iface_modem_3gpp_wait_for_packet_service_state        (MMIfaceModem3gpp              *self,
                                                                                         MMModem3gppPacketServiceState  final_state,
+                                                                                        GCancellable                  *cancellable,
                                                                                         GAsyncReadyCallback            callback,
                                                                                         gpointer                       user_data);
 MMModem3gppPacketServiceState mm_iface_modem_3gpp_wait_for_packet_service_state_finish (MMIfaceModem3gpp  *self,
diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c
index 245ae6606..635d915d4 100644
--- a/src/mm-iface-modem-simple.c
+++ b/src/mm-iface-modem-simple.c
@@ -228,7 +228,7 @@ typedef struct {
 static void
 packet_service_attach_in_3gpp_network_context_free (PacketServiceAttachIn3gppNetworkContext *ctx)
 {
-    g_assert (!ctx->error);
+    g_clear_error (&ctx->error);
     g_slice_free (PacketServiceAttachIn3gppNetworkContext, ctx);
 }
 
@@ -296,6 +296,11 @@ packet_service_attach_in_3gpp_network_step (GTask *task)
     self = g_task_get_source_object (task);
     ctx = g_task_get_task_data (task);
 
+    if (g_task_return_error_if_cancelled (task)) {
+        g_object_unref (task);
+        return;
+    }
+
     switch (ctx->step) {
         case PACKET_SERVICE_ATTACH_IN_3GPP_NETWORK_STEP_FIRST:
             ctx->step++;
@@ -305,6 +310,7 @@ packet_service_attach_in_3gpp_network_step (GTask *task)
             g_assert (!ctx->error);
             mm_iface_modem_3gpp_wait_for_packet_service_state (MM_IFACE_MODEM_3GPP (self),
                                                                MM_MODEM_3GPP_PACKET_SERVICE_STATE_ATTACHED,
+                                                               g_task_get_cancellable (task),
                                                                (GAsyncReadyCallback)wait_for_packet_service_state_ready,
                                                                task);
             return;
@@ -322,6 +328,7 @@ packet_service_attach_in_3gpp_network_step (GTask *task)
             g_assert (!ctx->error);
             mm_iface_modem_3gpp_wait_for_packet_service_state (MM_IFACE_MODEM_3GPP (self),
                                                                MM_MODEM_3GPP_PACKET_SERVICE_STATE_ATTACHED,
+                                                               g_task_get_cancellable (task),
                                                                (GAsyncReadyCallback)wait_for_packet_service_state_ready,
                                                                task);
             return;
@@ -341,13 +348,14 @@ packet_service_attach_in_3gpp_network_step (GTask *task)
 
 static void
 packet_service_attach_in_3gpp_network (MMIfaceModemSimple  *self,
+                                       GCancellable        *cancellable,
                                        GAsyncReadyCallback  callback,
                                        gpointer             user_data)
 {
     PacketServiceAttachIn3gppNetworkContext *ctx;
     GTask                                   *task;
 
-    task = g_task_new (self, NULL, callback, user_data);
+    task = g_task_new (self, cancellable, callback, user_data);
     ctx = g_slice_new0 (PacketServiceAttachIn3gppNetworkContext);
     ctx->step = PACKET_SERVICE_ATTACH_IN_3GPP_NETWORK_STEP_FIRST;
     g_task_set_task_data (task, ctx, (GDestroyNotify)packet_service_attach_in_3gpp_network_context_free);
@@ -777,6 +785,7 @@ connection_step (ConnectionContext *ctx)
         if (mm_iface_modem_is_3gpp (MM_IFACE_MODEM (ctx->self))) {
             packet_service_attach_in_3gpp_network (
                 ctx->self,
+                ctx->cancellable,
                 (GAsyncReadyCallback)packet_service_attach_in_3gpp_network_ready,
                 ctx);
             return;