From acc98e43ba3aeea8d8be1a92d009f775a8cd83f4 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Fri, 14 Apr 2023 12:03:32 +0200 Subject: 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. --- src/mm-iface-modem-3gpp.c | 95 ++++++++++++++++++++++++++++++++++++--------- src/mm-iface-modem-3gpp.h | 1 + src/mm-iface-modem-simple.c | 13 ++++++- 3 files changed, 88 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index c9e109e9..f6f8c881 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 5054570f..424e33eb 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 245ae660..635d915d 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; -- cgit v1.2.3-70-g09d2