broadband-modem: update modem state before enabling task is completed

The modem state property update was happening at enabling task context
free time, after the actual task completion has happened. Due to this,
scheduling a new enable request right after finishing one would end up
with the logic asserting, as the new enable operation would be
scheduled while the modem was still reporting "enabling" state.

  ModemManager[7673]: <inf> [1722359278.131123] [modem0] processing user request to enable modem...
  **
  ERROR:../src/mm-broadband-modem.c:12487:enable: code should not be reached
  Bail out! ERROR:../src/mm-broadband-modem.c:12487:enable: code should not be reached
  Aborted
This commit is contained in:
Aleksander Morgado
2024-09-30 07:51:53 +00:00
committed by Aleksander Morgado
parent a030eaef76
commit 17929188d8
+122 -113
View File
@@ -12081,33 +12081,16 @@ typedef enum {
} EnablingStep;
typedef struct {
MMBroadbandModem *self;
EnablingStep step;
MMModemState previous_state;
gboolean enabled;
GError *saved_error;
EnablingStep step;
MMModemState previous_state;
GError *saved_error;
} EnablingContext;
static void enabling_step (GTask *task);
static void
enabling_context_free (EnablingContext *ctx)
{
g_assert (!ctx->saved_error);
if (ctx->enabled)
mm_iface_modem_update_state (MM_IFACE_MODEM (ctx->self),
MM_MODEM_STATE_ENABLED,
MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED);
else if (ctx->previous_state != MM_MODEM_STATE_ENABLED) {
/* Fallback to previous state */
mm_iface_modem_update_state (MM_IFACE_MODEM (ctx->self),
ctx->previous_state,
MM_MODEM_STATE_CHANGE_REASON_UNKNOWN);
}
g_object_unref (ctx->self);
g_free (ctx);
g_slice_free (EnablingContext, ctx);
}
static gboolean
@@ -12118,6 +12101,36 @@ enable_finish (MMBaseModem *self,
return g_task_propagate_boolean (G_TASK (res), error);
}
static void
enabling_complete (GTask *task)
{
MMBroadbandModem *self;
EnablingContext *ctx;
self = g_task_get_source_object (task);
ctx = g_task_get_task_data (task);
/* Enable failed? */
if (ctx->saved_error) {
if (ctx->previous_state != MM_MODEM_STATE_ENABLED) {
/* Fallback to previous state */
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;
}
/* Enable succeeded */
mm_iface_modem_update_state (MM_IFACE_MODEM (self),
MM_MODEM_STATE_ENABLED,
MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED);
g_task_return_boolean (task, TRUE);
g_object_unref (task);
}
static void
enable_failed_ready (MMBroadbandModem *self,
GAsyncResult *res,
@@ -12125,16 +12138,16 @@ enable_failed_ready (MMBroadbandModem *self,
{
EnablingContext *ctx;
ctx = g_task_get_task_data (task);
/* The disabling run after a failed enable will never fail */
g_assert (enable_failed_finish (self, res, NULL));
enable_failed_finish (self, res, NULL);
ctx = g_task_get_task_data (task);
g_assert (ctx->saved_error);
g_task_return_error (task, g_steal_pointer (&ctx->saved_error));
g_object_unref (task);
enabling_complete (task);
}
static void enabling_step (GTask *task);
#undef INTERFACE_ENABLE_READY_FN
#define INTERFACE_ENABLE_READY_FN(NAME,TYPE,FATAL_ERRORS) \
static void \
@@ -12179,20 +12192,19 @@ INTERFACE_ENABLE_READY_FN (iface_modem_oma, MM_IFACE_MODEM_OMA,
static void
enabling_started_ready (MMBroadbandModem *self,
GAsyncResult *result,
GTask *task)
GAsyncResult *result,
GTask *task)
{
EnablingContext *ctx;
GError *error = NULL;
if (!MM_BROADBAND_MODEM_GET_CLASS (self)->enabling_started_finish (self, result, &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_BROADBAND_MODEM_GET_CLASS (self)->enabling_started_finish (self, result, &ctx->saved_error)) {
enabling_complete (task);
return;
}
/* Go on to next step */
ctx = g_task_get_task_data (task);
ctx->step++;
enabling_step (task);
}
@@ -12203,27 +12215,24 @@ enabling_wait_for_final_state_ready (MMIfaceModem *self,
GTask *task)
{
EnablingContext *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) {
enabling_complete (task);
return;
}
if (ctx->previous_state >= MM_MODEM_STATE_ENABLED) {
/* Just return success, don't relaunch enabling */
g_task_return_boolean (task, TRUE);
g_object_unref (task);
enabling_complete (task);
return;
}
/* We're in a final state now, go on */
mm_iface_modem_update_state (MM_IFACE_MODEM (ctx->self),
mm_iface_modem_update_state (MM_IFACE_MODEM (self),
MM_MODEM_STATE_ENABLING,
MM_MODEM_STATE_CHANGE_REASON_USER_REQUESTED);
@@ -12234,34 +12243,37 @@ enabling_wait_for_final_state_ready (MMIfaceModem *self,
static void
enabling_step (GTask *task)
{
EnablingContext *ctx;
MMBroadbandModem *self;
EnablingContext *ctx;
self = g_task_get_source_object (task);
ctx = g_task_get_task_data (task);
g_assert (!ctx->saved_error);
/* Don't run new steps if we're cancelled */
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)) {
enabling_complete (task);
return;
}
ctx = g_task_get_task_data (task);
switch (ctx->step) {
case ENABLING_STEP_FIRST:
ctx->step++;
/* fall through */
case ENABLING_STEP_WAIT_FOR_FINAL_STATE:
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)enabling_wait_for_final_state_ready,
task);
return;
case ENABLING_STEP_STARTED:
if (MM_BROADBAND_MODEM_GET_CLASS (ctx->self)->enabling_started &&
MM_BROADBAND_MODEM_GET_CLASS (ctx->self)->enabling_started_finish) {
MM_BROADBAND_MODEM_GET_CLASS (ctx->self)->enabling_started (ctx->self,
(GAsyncReadyCallback)enabling_started_ready,
task);
if (MM_BROADBAND_MODEM_GET_CLASS (self)->enabling_started &&
MM_BROADBAND_MODEM_GET_CLASS (self)->enabling_started_finish) {
MM_BROADBAND_MODEM_GET_CLASS (self)->enabling_started (self,
(GAsyncReadyCallback)enabling_started_ready,
task);
return;
}
ctx->step++;
@@ -12271,19 +12283,19 @@ enabling_step (GTask *task)
/* From now on, the failure to enable one of the mandatory interfaces
* will trigger the implicit disabling process */
g_assert (ctx->self->priv->modem_dbus_skeleton != NULL);
g_assert (self->priv->modem_dbus_skeleton != NULL);
/* Enabling the Modem interface */
mm_iface_modem_enable (MM_IFACE_MODEM (ctx->self),
mm_iface_modem_enable (MM_IFACE_MODEM (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_enable_ready,
task);
return;
case ENABLING_STEP_IFACE_3GPP:
if (ctx->self->priv->modem_3gpp_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has 3GPP capabilities, enabling the Modem 3GPP interface...");
if (self->priv->modem_3gpp_dbus_skeleton) {
mm_obj_dbg (self, "modem has 3GPP capabilities, enabling the Modem 3GPP interface...");
/* Enabling the Modem 3GPP interface */
mm_iface_modem_3gpp_enable (MM_IFACE_MODEM_3GPP (ctx->self),
mm_iface_modem_3gpp_enable (MM_IFACE_MODEM_3GPP (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_3gpp_enable_ready,
task);
@@ -12293,9 +12305,9 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_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, enabling the Modem 3GPP Profile Manager interface...");
mm_iface_modem_3gpp_profile_manager_enable (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, enabling the Modem 3GPP Profile Manager interface...");
mm_iface_modem_3gpp_profile_manager_enable (MM_IFACE_MODEM_3GPP_PROFILE_MANAGER (self),
(GAsyncReadyCallback)iface_modem_3gpp_profile_manager_enable_ready,
task);
return;
@@ -12304,9 +12316,9 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_3GPP_USSD:
if (ctx->self->priv->modem_3gpp_ussd_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has 3GPP/USSD capabilities, enabling the Modem 3GPP/USSD interface...");
mm_iface_modem_3gpp_ussd_enable (MM_IFACE_MODEM_3GPP_USSD (ctx->self),
if (self->priv->modem_3gpp_ussd_dbus_skeleton) {
mm_obj_dbg (self, "modem has 3GPP/USSD capabilities, enabling the Modem 3GPP/USSD interface...");
mm_iface_modem_3gpp_ussd_enable (MM_IFACE_MODEM_3GPP_USSD (self),
(GAsyncReadyCallback)iface_modem_3gpp_ussd_enable_ready,
task);
return;
@@ -12315,10 +12327,10 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_CDMA:
if (ctx->self->priv->modem_cdma_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has CDMA capabilities, enabling the Modem CDMA interface...");
if (self->priv->modem_cdma_dbus_skeleton) {
mm_obj_dbg (self, "modem has CDMA capabilities, enabling the Modem CDMA interface...");
/* Enabling the Modem CDMA interface */
mm_iface_modem_cdma_enable (MM_IFACE_MODEM_CDMA (ctx->self),
mm_iface_modem_cdma_enable (MM_IFACE_MODEM_CDMA (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_cdma_enable_ready,
task);
@@ -12328,10 +12340,10 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_LOCATION:
if (ctx->self->priv->modem_location_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has location capabilities, enabling the Location interface...");
if (self->priv->modem_location_dbus_skeleton) {
mm_obj_dbg (self, "modem has location capabilities, enabling the Location interface...");
/* Enabling the Modem Location interface */
mm_iface_modem_location_enable (MM_IFACE_MODEM_LOCATION (ctx->self),
mm_iface_modem_location_enable (MM_IFACE_MODEM_LOCATION (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_location_enable_ready,
task);
@@ -12341,10 +12353,10 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_MESSAGING:
if (ctx->self->priv->modem_messaging_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has messaging capabilities, enabling the Messaging interface...");
if (self->priv->modem_messaging_dbus_skeleton) {
mm_obj_dbg (self, "modem has messaging capabilities, enabling the Messaging interface...");
/* Enabling the Modem Messaging interface */
mm_iface_modem_messaging_enable (MM_IFACE_MODEM_MESSAGING (ctx->self),
mm_iface_modem_messaging_enable (MM_IFACE_MODEM_MESSAGING (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_messaging_enable_ready,
task);
@@ -12354,10 +12366,10 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_TIME:
if (ctx->self->priv->modem_time_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has time capabilities, enabling the Time interface...");
if (self->priv->modem_time_dbus_skeleton) {
mm_obj_dbg (self, "modem has time capabilities, enabling the Time interface...");
/* Enabling the Modem Time interface */
mm_iface_modem_time_enable (MM_IFACE_MODEM_TIME (ctx->self),
mm_iface_modem_time_enable (MM_IFACE_MODEM_TIME (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_time_enable_ready,
task);
@@ -12367,10 +12379,10 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_SIGNAL:
if (ctx->self->priv->modem_signal_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has extended signal reporting capabilities, enabling the Signal interface...");
if (self->priv->modem_signal_dbus_skeleton) {
mm_obj_dbg (self, "modem has extended signal reporting capabilities, enabling the Signal interface...");
/* Enabling the Modem Signal interface */
mm_iface_modem_signal_enable (MM_IFACE_MODEM_SIGNAL (ctx->self),
mm_iface_modem_signal_enable (MM_IFACE_MODEM_SIGNAL (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_signal_enable_ready,
task);
@@ -12380,10 +12392,10 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_OMA:
if (ctx->self->priv->modem_oma_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has OMA capabilities, enabling the OMA interface...");
if (self->priv->modem_oma_dbus_skeleton) {
mm_obj_dbg (self, "modem has OMA capabilities, enabling the OMA interface...");
/* Enabling the Modem Oma interface */
mm_iface_modem_oma_enable (MM_IFACE_MODEM_OMA (ctx->self),
mm_iface_modem_oma_enable (MM_IFACE_MODEM_OMA (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_oma_enable_ready,
task);
@@ -12393,10 +12405,10 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_IFACE_VOICE:
if (ctx->self->priv->modem_voice_dbus_skeleton) {
mm_obj_dbg (ctx->self, "modem has voice capabilities, enabling the Voice interface...");
if (self->priv->modem_voice_dbus_skeleton) {
mm_obj_dbg (self, "modem has voice capabilities, enabling the Voice interface...");
/* Enabling the Modem Voice interface */
mm_iface_modem_voice_enable (MM_IFACE_MODEM_VOICE (ctx->self),
mm_iface_modem_voice_enable (MM_IFACE_MODEM_VOICE (self),
g_task_get_cancellable (task),
(GAsyncReadyCallback)iface_modem_voice_enable_ready,
task);
@@ -12414,8 +12426,6 @@ enabling_step (GTask *task)
/* fall through */
case ENABLING_STEP_LAST:
ctx->enabled = TRUE;
/* Once all interfaces have been enabled, trigger registration checks in
* 3GPP and CDMA modems. We have to do this at this point so that e.g.
* location interface gets proper registration related info reported.
@@ -12423,11 +12433,10 @@ enabling_step (GTask *task)
* We do this in an idle so that we don't mess up the logs at this point
* with the new requests being triggered.
*/
schedule_initial_registration_checks (ctx->self);
schedule_initial_registration_checks (self);
/* All enabled without errors! */
g_task_return_boolean (task, TRUE);
g_object_unref (task);
enabling_complete (task);
return;
default:
@@ -12438,10 +12447,22 @@ enabling_step (GTask *task)
}
static void
enable (MMBaseModem *self,
GCancellable *cancellable,
GAsyncReadyCallback callback,
gpointer user_data)
enabling_start (GTask *task)
{
EnablingContext *ctx;
ctx = g_slice_new0 (EnablingContext);
ctx->step = ENABLING_STEP_FIRST;
g_task_set_task_data (task, ctx, (GDestroyNotify)enabling_context_free);
enabling_step (task);
}
static void
enable (MMBaseModem *self,
GCancellable *cancellable,
GAsyncReadyCallback callback,
gpointer user_data)
{
GTask *task;
@@ -12455,33 +12476,22 @@ enable (MMBaseModem *self,
break;
case MM_MODEM_STATE_FAILED:
g_task_return_new_error (task,
MM_CORE_ERROR,
MM_CORE_ERROR_WRONG_STATE,
g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_WRONG_STATE,
"Cannot enable modem: initialization failed");
g_object_unref (task);
break;
case MM_MODEM_STATE_LOCKED:
g_task_return_new_error (task,
MM_CORE_ERROR,
MM_CORE_ERROR_WRONG_STATE,
g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_WRONG_STATE,
"Cannot enable modem: device locked");
g_object_unref (task);
break;
case MM_MODEM_STATE_INITIALIZING:
case MM_MODEM_STATE_DISABLED:
case MM_MODEM_STATE_DISABLING: {
EnablingContext *ctx;
ctx = g_new0 (EnablingContext, 1);
ctx->self = MM_BROADBAND_MODEM (g_object_ref (self));
ctx->step = ENABLING_STEP_FIRST;
g_task_set_task_data (task, ctx, (GDestroyNotify)enabling_context_free);
enabling_step (task);
case MM_MODEM_STATE_DISABLING:
enabling_start (task);
return;
}
case MM_MODEM_STATE_ENABLING:
g_assert_not_reached ();
@@ -12495,13 +12505,12 @@ enable (MMBaseModem *self,
case MM_MODEM_STATE_CONNECTED:
/* Just return success, don't relaunch enabling */
g_task_return_boolean (task, TRUE);
g_object_unref (task);
break;
default:
g_assert_not_reached ();
}
g_object_unref (task);
}
/*****************************************************************************/