From 8e8e88b75df329eb065e11491bbe5420c75cfd4e Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Tue, 30 Jul 2024 21:53:39 +0000 Subject: broadband-modem: update modem state before disabling task is completed The modem state property update was happening at disabling task context free time, after the actual task completion has happened. Avoid wrong assumptions on the modem state value in operations scheduled after the task completion, in the same way as done in the enabling sequence. --- src/mm-broadband-modem.c | 192 +++++++++++++++++++++++++---------------------- 1 file changed, 102 insertions(+), 90 deletions(-) (limited to 'src') diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 22313663..79f6496a 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -11631,51 +11631,65 @@ typedef enum { } DisablingStep; typedef struct { - MMBroadbandModem *self; - gboolean state_updates; - DisablingStep step; - MMModemState previous_state; - gboolean disabled; + gboolean state_updates; + DisablingStep step; + MMModemState previous_state; + GError *saved_error; } DisablingContext; -static void disabling_step (GTask *task); - static void disabling_context_free (DisablingContext *ctx) { - GError *error = NULL; + g_assert (!ctx->saved_error); + g_slice_free (DisablingContext, ctx); +} - if (MM_BROADBAND_MODEM_GET_CLASS (ctx->self)->disabling_stopped && - !MM_BROADBAND_MODEM_GET_CLASS (ctx->self)->disabling_stopped (ctx->self, &error)) { - mm_obj_warn (ctx->self, "error when stopping the disabling sequence: %s", error->message); - g_error_free (error); +static gboolean +common_disable_finish (MMBroadbandModem *self, + GAsyncResult *res, + GError **error) +{ + return g_task_propagate_boolean (G_TASK (res), error); +} + +static void +disabling_complete (GTask *task) +{ + MMBroadbandModem *self; + DisablingContext *ctx; + g_autoptr(GError) error = NULL; + + self = g_task_get_source_object (task); + ctx = g_task_get_task_data (task); + + if (MM_BROADBAND_MODEM_GET_CLASS (self)->disabling_stopped && + !MM_BROADBAND_MODEM_GET_CLASS (self)->disabling_stopped (self, &error)) { + mm_obj_warn (self, "error when stopping the disabling sequence: %s", error->message); } - /* Only perform state updates if we're asked to do so */ - if (ctx->state_updates) { - if (ctx->disabled) - mm_iface_modem_update_state (MM_IFACE_MODEM (ctx->self), - MM_MODEM_STATE_DISABLED, - MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED); - else if (ctx->previous_state != MM_MODEM_STATE_DISABLED) { + /* Disable failed? */ + if (ctx->saved_error) { + if (ctx->state_updates && (ctx->previous_state != MM_MODEM_STATE_DISABLED)) { /* Fallback to previous state */ - mm_iface_modem_update_state (MM_IFACE_MODEM (ctx->self), + mm_iface_modem_update_state (MM_IFACE_MODEM (self), ctx->previous_state, MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); } + g_task_return_error (task, g_steal_pointer (&ctx->saved_error)); + g_object_unref (task); + return; } - g_object_unref (ctx->self); - g_free (ctx); + /* Disable succeeded */ + if (ctx->state_updates) + mm_iface_modem_update_state (MM_IFACE_MODEM (self), + MM_MODEM_STATE_DISABLED, + MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED); + g_task_return_boolean (task, TRUE); + g_object_unref (task); } -static gboolean -common_disable_finish (MMBroadbandModem *self, - GAsyncResult *res, - GError **error) -{ - return g_task_propagate_boolean (G_TASK (res), error); -} +static void disabling_step (GTask *task); #undef INTERFACE_DISABLE_READY_FN #define INTERFACE_DISABLE_READY_FN(NAME,TYPE,WARN_ERRORS) \ @@ -11718,11 +11732,11 @@ bearer_list_disconnect_bearers_ready (MMBearerList *list, GTask *task) { DisablingContext *ctx; - GError *error = NULL; - if (!mm_bearer_list_disconnect_bearers_finish (list, res, &error)) { - g_task_return_error (task, error); - g_object_unref (task); + ctx = g_task_get_task_data (task); + g_assert (!ctx->saved_error); + if (!mm_bearer_list_disconnect_bearers_finish (list, res, &ctx->saved_error)) { + disabling_complete (task); return; } @@ -11738,14 +11752,12 @@ disabling_wait_for_final_state_ready (MMIfaceModem *self, GTask *task) { DisablingContext *ctx; - GError *error = NULL; ctx = g_task_get_task_data (task); - - ctx->previous_state = mm_iface_modem_wait_for_final_state_finish (self, res, &error); - if (error) { - g_task_return_error (task, error); - g_object_unref (task); + g_assert (!ctx->saved_error); + ctx->previous_state = mm_iface_modem_wait_for_final_state_finish (self, res, &ctx->saved_error); + if (ctx->saved_error) { + disabling_complete (task); return; } @@ -11758,8 +11770,7 @@ disabling_wait_for_final_state_ready (MMIfaceModem *self, * Note that we do consider here UNKNOWN and FAILED status on purpose, * as the MMManager will try to disable every modem before removing * it. */ - g_task_return_boolean (task, TRUE); - g_object_unref (task); + disabling_complete (task); return; case MM_MODEM_STATE_INITIALIZING: case MM_MODEM_STATE_DISABLING: @@ -11777,7 +11788,7 @@ disabling_wait_for_final_state_ready (MMIfaceModem *self, /* We're in a final state now, go on */ g_assert (ctx->state_updates); - mm_iface_modem_update_state (MM_IFACE_MODEM (ctx->self), + mm_iface_modem_update_state (MM_IFACE_MODEM (self), MM_MODEM_STATE_DISABLING, MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED); @@ -11788,9 +11799,12 @@ disabling_wait_for_final_state_ready (MMIfaceModem *self, static void disabling_step (GTask *task) { + MMBroadbandModem *self; DisablingContext *ctx; - ctx = g_task_get_task_data (task); + self = g_task_get_source_object (task); + ctx = g_task_get_task_data (task); + g_assert (!ctx->saved_error); switch (ctx->step) { case DISABLING_STEP_FIRST: @@ -11801,17 +11815,18 @@ disabling_step (GTask *task) /* Connection requests via the Simple interface must be aborted as soon * as possible, because certain steps may be explicitly waiting for new * state transitions and such. */ - mm_iface_modem_simple_abort_ongoing (MM_IFACE_MODEM_SIMPLE (ctx->self)); + mm_iface_modem_simple_abort_ongoing (MM_IFACE_MODEM_SIMPLE (self)); ctx->step++; /* fall through */ case DISABLING_STEP_WAIT_FOR_FINAL_STATE: /* cancellability allowed at this point */ - if (g_task_return_error_if_cancelled (task)) { - g_object_unref (task); + if (g_cancellable_set_error_if_cancelled (g_task_get_cancellable (task), &ctx->saved_error)) { + disabling_complete (task); return; } - mm_iface_modem_wait_for_final_state (MM_IFACE_MODEM (ctx->self), + + mm_iface_modem_wait_for_final_state (MM_IFACE_MODEM (self), MM_MODEM_STATE_UNKNOWN, /* just any */ (GAsyncReadyCallback)disabling_wait_for_final_state_ready, task); @@ -11819,13 +11834,13 @@ disabling_step (GTask *task) case DISABLING_STEP_DISCONNECT_BEARERS: /* cancellability allowed at this point */ - if (g_task_return_error_if_cancelled (task)) { - g_object_unref (task); + if (g_cancellable_set_error_if_cancelled (g_task_get_cancellable (task), &ctx->saved_error)) { + disabling_complete (task); return; } - if (ctx->self->priv->modem_bearer_list) { + if (self->priv->modem_bearer_list) { mm_bearer_list_disconnect_bearers ( - ctx->self->priv->modem_bearer_list, + self->priv->modem_bearer_list, NULL, /* all bearers */ (GAsyncReadyCallback)bearer_list_disconnect_bearers_ready, task); @@ -11851,9 +11866,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_VOICE: - if (ctx->self->priv->modem_voice_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has voice capabilities, disabling the Voice interface..."); - mm_iface_modem_voice_disable (MM_IFACE_MODEM_VOICE (ctx->self), + if (self->priv->modem_voice_dbus_skeleton) { + mm_obj_dbg (self, "modem has voice capabilities, disabling the Voice interface..."); + mm_iface_modem_voice_disable (MM_IFACE_MODEM_VOICE (self), (GAsyncReadyCallback)iface_modem_voice_disable_ready, task); return; @@ -11862,9 +11877,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_SIGNAL: - if (ctx->self->priv->modem_signal_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has extended signal reporting capabilities, disabling the Signal interface..."); - mm_iface_modem_signal_disable (MM_IFACE_MODEM_SIGNAL (ctx->self), + if (self->priv->modem_signal_dbus_skeleton) { + mm_obj_dbg (self, "modem has extended signal reporting capabilities, disabling the Signal interface..."); + mm_iface_modem_signal_disable (MM_IFACE_MODEM_SIGNAL (self), (GAsyncReadyCallback)iface_modem_signal_disable_ready, task); return; @@ -11873,9 +11888,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_OMA: - if (ctx->self->priv->modem_oma_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has OMA capabilities, disabling the OMA interface..."); - mm_iface_modem_oma_disable (MM_IFACE_MODEM_OMA (ctx->self), + if (self->priv->modem_oma_dbus_skeleton) { + mm_obj_dbg (self, "modem has OMA capabilities, disabling the OMA interface..."); + mm_iface_modem_oma_disable (MM_IFACE_MODEM_OMA (self), (GAsyncReadyCallback)iface_modem_oma_disable_ready, task); return; @@ -11884,9 +11899,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_TIME: - if (ctx->self->priv->modem_time_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has time capabilities, disabling the Time interface..."); - mm_iface_modem_time_disable (MM_IFACE_MODEM_TIME (ctx->self), + if (self->priv->modem_time_dbus_skeleton) { + mm_obj_dbg (self, "modem has time capabilities, disabling the Time interface..."); + mm_iface_modem_time_disable (MM_IFACE_MODEM_TIME (self), (GAsyncReadyCallback)iface_modem_time_disable_ready, task); return; @@ -11895,9 +11910,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_MESSAGING: - if (ctx->self->priv->modem_messaging_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has messaging capabilities, disabling the Messaging interface..."); - mm_iface_modem_messaging_disable (MM_IFACE_MODEM_MESSAGING (ctx->self), + if (self->priv->modem_messaging_dbus_skeleton) { + mm_obj_dbg (self, "modem has messaging capabilities, disabling the Messaging interface..."); + mm_iface_modem_messaging_disable (MM_IFACE_MODEM_MESSAGING (self), (GAsyncReadyCallback)iface_modem_messaging_disable_ready, task); return; @@ -11906,9 +11921,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_LOCATION: - if (ctx->self->priv->modem_location_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has location capabilities, disabling the Location interface..."); - mm_iface_modem_location_disable (MM_IFACE_MODEM_LOCATION (ctx->self), + if (self->priv->modem_location_dbus_skeleton) { + mm_obj_dbg (self, "modem has location capabilities, disabling the Location interface..."); + mm_iface_modem_location_disable (MM_IFACE_MODEM_LOCATION (self), (GAsyncReadyCallback)iface_modem_location_disable_ready, task); return; @@ -11917,9 +11932,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_CDMA: - if (ctx->self->priv->modem_cdma_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has CDMA capabilities, disabling the Modem CDMA interface..."); - mm_iface_modem_cdma_disable (MM_IFACE_MODEM_CDMA (ctx->self), + if (self->priv->modem_cdma_dbus_skeleton) { + mm_obj_dbg (self, "modem has CDMA capabilities, disabling the Modem CDMA interface..."); + mm_iface_modem_cdma_disable (MM_IFACE_MODEM_CDMA (self), (GAsyncReadyCallback)iface_modem_cdma_disable_ready, task); return; @@ -11928,9 +11943,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_3GPP_USSD: - if (ctx->self->priv->modem_3gpp_ussd_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has 3GPP/USSD capabilities, disabling the Modem 3GPP/USSD interface..."); - mm_iface_modem_3gpp_ussd_disable (MM_IFACE_MODEM_3GPP_USSD (ctx->self), + if (self->priv->modem_3gpp_ussd_dbus_skeleton) { + mm_obj_dbg (self, "modem has 3GPP/USSD capabilities, disabling the Modem 3GPP/USSD interface..."); + mm_iface_modem_3gpp_ussd_disable (MM_IFACE_MODEM_3GPP_USSD (self), (GAsyncReadyCallback)iface_modem_3gpp_ussd_disable_ready, task); return; @@ -11939,9 +11954,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_3GPP_PROFILE_MANAGER: - if (ctx->self->priv->modem_3gpp_profile_manager_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has 3GPP profile management capabilities, disabling the Modem 3GPP Profile Manager interface..."); - mm_iface_modem_3gpp_profile_manager_disable (MM_IFACE_MODEM_3GPP_PROFILE_MANAGER (ctx->self), + if (self->priv->modem_3gpp_profile_manager_dbus_skeleton) { + mm_obj_dbg (self, "modem has 3GPP profile management capabilities, disabling the Modem 3GPP Profile Manager interface..."); + mm_iface_modem_3gpp_profile_manager_disable (MM_IFACE_MODEM_3GPP_PROFILE_MANAGER (self), (GAsyncReadyCallback)iface_modem_3gpp_profile_manager_disable_ready, task); return; @@ -11950,9 +11965,9 @@ disabling_step (GTask *task) /* fall through */ case DISABLING_STEP_IFACE_3GPP: - if (ctx->self->priv->modem_3gpp_dbus_skeleton) { - mm_obj_dbg (ctx->self, "modem has 3GPP capabilities, disabling the Modem 3GPP interface..."); - mm_iface_modem_3gpp_disable (MM_IFACE_MODEM_3GPP (ctx->self), + if (self->priv->modem_3gpp_dbus_skeleton) { + mm_obj_dbg (self, "modem has 3GPP capabilities, disabling the Modem 3GPP interface..."); + mm_iface_modem_3gpp_disable (MM_IFACE_MODEM_3GPP (self), (GAsyncReadyCallback)iface_modem_3gpp_disable_ready, task); return; @@ -11963,9 +11978,9 @@ disabling_step (GTask *task) case DISABLING_STEP_IFACE_MODEM: /* This skeleton may be NULL when mm_base_modem_disable() gets called at * the same time as modem object disposal. */ - if (ctx->self->priv->modem_dbus_skeleton) { - mm_obj_dbg (ctx->self, "disabling the Modem interface..."); - mm_iface_modem_disable (MM_IFACE_MODEM (ctx->self), + if (self->priv->modem_dbus_skeleton) { + mm_obj_dbg (self, "disabling the Modem interface..."); + mm_iface_modem_disable (MM_IFACE_MODEM (self), (GAsyncReadyCallback)iface_modem_disable_ready, task); return; @@ -11975,9 +11990,7 @@ disabling_step (GTask *task) case DISABLING_STEP_LAST: /* All disabled without errors! */ - ctx->disabled = TRUE; - g_task_return_boolean (task, TRUE); - g_object_unref (task); + disabling_complete (task); return; default: @@ -11998,8 +12011,7 @@ common_disable (MMBroadbandModem *self, DisablingContext *ctx; GTask *task; - ctx = g_new0 (DisablingContext, 1); - ctx->self = g_object_ref (self); + ctx = g_slice_new0 (DisablingContext); ctx->state_updates = state_updates; ctx->step = first_step; @@ -12017,7 +12029,7 @@ enable_failed_finish (MMBroadbandModem *self, GError **error) { /* The implicit disabling should never ever fail */ - g_assert (common_disable_finish (self, res, NULL)); + common_disable_finish (self, res, NULL); return TRUE; } -- cgit v1.2.3-70-g09d2