Merge request !1311 from 'sleep-wait'

Wait until  device operations complete before dropping suspend inhibitor

https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/merge_requests/1311

Closes #966

1 files changed, 268 insertions, 113 deletions

diff --git a/src/mm-base-manager.c b/src/mm-base-manager.c
index ce9eb032..f473ef2a 100644
--- a/src/mm-base-manager.c
+++ b/src/mm-base-manager.c
@@ -130,23 +130,6 @@ struct _MMBaseManagerPrivate {
 /*****************************************************************************/
 
 static MMDevice *
-find_device_by_modem (MMBaseManager *manager,
-                      MMBaseModem *modem)
-{
-    GHashTableIter iter;
-    gpointer key, value;
-
-    g_hash_table_iter_init (&iter, manager->priv->devices);
-    while (g_hash_table_iter_next (&iter, &key, &value)) {
-        MMDevice *candidate = MM_DEVICE (value);
-
-        if (modem == mm_device_peek_modem (candidate))
-            return candidate;
-    }
-    return NULL;
-}
-
-static MMDevice *
 find_device_by_port (MMBaseManager  *manager,
                      MMKernelDevice *port)
 {
@@ -367,7 +350,7 @@ device_removed (MMBaseManager *self,
 
         /* The device may have already been removed from the tracking HT, we
          * just try to remove it and if it fails, we ignore it */
-        mm_device_remove_modem (device);
+        mm_device_remove_modem_quick (device);
         g_hash_table_remove (self->priv->devices, mm_device_get_uid (device));
     }
 }
@@ -456,6 +439,19 @@ existing_port (MMBaseManager  *self,
 }
 
 static void
+additional_port_modem_remove_ready (MMDevice      *device,
+                                    GAsyncResult  *res,
+                                    MMBaseManager *self)
+{
+    g_autoptr(GError) error = NULL;
+
+    if (!mm_device_remove_modem_finish (device, res, &error))
+        mm_obj_warn (device, "removing modem failed: %s", error->message);
+    device_support_check_add_all_ports (self, device);
+    g_object_unref (self);
+}
+
+static void
 additional_port (MMBaseManager  *self,
                  MMDevice       *device,
                  MMKernelDevice *port)
@@ -500,9 +496,9 @@ additional_port (MMBaseManager  *self,
     }
 
     mm_obj_info (self, "last modem object creation in device '%s' succeeded, but we have a new port addition, will retry", uid);
-    g_cancellable_cancel (mm_base_modem_peek_cancellable (modem));
-    mm_device_remove_modem (device);
-    device_support_check_add_all_ports (self, device);
+    mm_device_remove_modem (device,
+                            (GAsyncReadyCallback)additional_port_modem_remove_ready,
+                            g_object_ref (self));
 }
 
 static void
@@ -873,146 +869,305 @@ mm_base_manager_start (MMBaseManager *self,
 
 /*****************************************************************************/
 
+typedef enum {
+    DEVICE_SHUTDOWN_STEP_FIRST = 0,
+    DEVICE_SHUTDOWN_STEP_DISABLE,
+    DEVICE_SHUTDOWN_STEP_LOW_POWER,
+    DEVICE_SHUTDOWN_STEP_REMOVE,
+    DEVICE_SHUTDOWN_STEP_LAST,
+} DeviceShutdownStep;
+
 typedef struct {
-    MMBaseManager *self;
-    gboolean       low_power;
-    gboolean       remove;
-} DisableContext;
+    MMBaseModem *modem;
+    gboolean     disable;
+    gboolean     low_power;
+    gboolean     remove;
+
+    DeviceShutdownStep step;
+} DeviceShutdownContext;
+
+static void device_shutdown_step (GTask *task);
+
+static DeviceShutdownContext *
+device_shutdown_context_new (MMBaseModem *modem,
+                             gboolean     disable,
+                             gboolean     low_power,
+                             gboolean     remove)
+{
+    DeviceShutdownContext *ctx;
+
+    ctx = g_slice_new0 (DeviceShutdownContext);
+    if (modem)
+        ctx->modem = g_object_ref (modem);
+    ctx->step = DEVICE_SHUTDOWN_STEP_FIRST;
+    ctx->disable = disable;
+    ctx->low_power = low_power;
+    ctx->remove = remove;
+
+    return ctx;
+}
 
 static void
-disable_context_free (DisableContext *ctx)
+device_shutdown_context_free (DeviceShutdownContext *ctx)
 {
-    g_object_unref (ctx->self);
-    g_slice_free (DisableContext, ctx);
+    g_clear_object (&ctx->modem);
+    g_slice_free (DeviceShutdownContext, ctx);
 }
 
 static void
-remove_device_after_disable (MMBaseModem    *modem,
-                             DisableContext *ctx)
+modem_remove_ready (MMDevice     *device,
+                    GAsyncResult *res,
+                    GTask        *task)
 {
-    MMDevice *device;
+    DeviceShutdownContext *ctx;
+    g_autoptr(GError)      error = NULL;
 
-    device = find_device_by_modem (ctx->self, modem);
-    if (device) {
-        g_cancellable_cancel (mm_base_modem_peek_cancellable (modem));
-        mm_device_remove_modem (device);
-        g_hash_table_remove (ctx->self->priv->devices, mm_device_get_uid (device));
-    }
+    ctx = g_task_get_task_data (task);
+
+    if (!mm_device_remove_modem_finish (device, res, &error))
+        mm_obj_warn (device, "removing modem failed: %s", error->message);
 
-    disable_context_free (ctx);
+    ctx->step++;
+    device_shutdown_step (task);
 }
 
 static void
-shutdown_low_power_ready (MMIfaceModem   *modem,
-                          GAsyncResult   *res,
-                          DisableContext *ctx)
+shutdown_low_power_ready (MMIfaceModem *modem,
+                          GAsyncResult *res,
+                          GTask        *task)
 {
-    g_autoptr(GError) error = NULL;
+    DeviceShutdownContext *ctx;
+    g_autoptr(GError)      error = NULL;
+
+    ctx = g_task_get_task_data (task);
 
     if (!mm_iface_modem_set_power_state_finish (modem, res, NULL, &error))
-        mm_obj_info (ctx->self, "changing to low power state failed: %s", error->message);
+        mm_obj_info (modem, "changing to low power state failed: %s", error->message);
 
-    if (ctx->remove)
-        remove_device_after_disable (MM_BASE_MODEM (modem), ctx);
-    else
-        disable_context_free (ctx);
+    ctx->step++;
+    device_shutdown_step (task);
 }
 
 static void
-shutdown_disable_ready (MMBaseModem    *modem,
-                        GAsyncResult   *res,
-                        DisableContext *ctx)
+shutdown_disable_ready (MMBaseModem  *modem,
+                        GAsyncResult *res,
+                        GTask        *task)
 {
-    g_autoptr(GError) error = NULL;
+    DeviceShutdownContext *ctx;
+    g_autoptr(GError)      error = NULL;
 
-    /* We don't care about errors disabling at this point */
-    if (!mm_base_modem_disable_finish (modem, res, &error)) {
-        mm_obj_info (ctx->self, "disabling modem failed: %s", error->message);
+    ctx = g_task_get_task_data (task);
+
+    if (mm_base_modem_disable_finish (modem, res, &error)) {
+        ctx->step++;
+    } else {
+        mm_obj_info (modem, "disabling modem failed: %s", error->message);
+
+        /* If disabling failed there's no point in trying to low-power the
+         * modem; just remove it.
+         */
+        ctx->step = DEVICE_SHUTDOWN_STEP_REMOVE;
     }
-    /* Bring the modem to low power mode if requested */
-    else if (ctx->low_power) {
-        mm_iface_modem_set_power_state (MM_IFACE_MODEM (modem),
-                                        MM_MODEM_POWER_STATE_LOW,
-                                        (GAsyncReadyCallback)shutdown_low_power_ready,
-                                        ctx);
+
+    device_shutdown_step (task);
+}
+
+static void
+device_shutdown_step (GTask *task)
+{
+    DeviceShutdownContext *ctx;
+    MMDevice              *device;
+
+    ctx = g_task_get_task_data (task);
+    device = g_task_get_source_object (task);
+
+    switch (ctx->step) {
+    case DEVICE_SHUTDOWN_STEP_FIRST:
+        ctx->step++;
+        /* fall through */
+
+    case DEVICE_SHUTDOWN_STEP_DISABLE:
+        if (ctx->disable && ctx->modem) {
+            mm_obj_dbg (device,
+                        "disabling modem%d",
+                        mm_base_modem_get_dbus_id (ctx->modem));
+            mm_base_modem_disable (ctx->modem,
+                                   MM_BASE_MODEM_OPERATION_LOCK_REQUIRED,
+                                   MM_BASE_MODEM_OPERATION_PRIORITY_OVERRIDE,
+                                   (GAsyncReadyCallback)shutdown_disable_ready,
+                                   task);
+            return;
+        }
+        ctx->step++;
+        /* fall through */
+
+    case DEVICE_SHUTDOWN_STEP_LOW_POWER:
+        if (ctx->low_power && ctx->modem) {
+            mm_obj_dbg (device,
+                        "setting modem%d to low-power state",
+                        mm_base_modem_get_dbus_id (ctx->modem));
+            mm_iface_modem_set_power_state (MM_IFACE_MODEM (ctx->modem),
+                                            MM_MODEM_POWER_STATE_LOW,
+                                            (GAsyncReadyCallback)shutdown_low_power_ready,
+                                            task);
+            return;
+        }
+        ctx->step++;
+        /* fall through */
+
+    case DEVICE_SHUTDOWN_STEP_REMOVE:
+        if (ctx->remove && ctx->modem) {
+            mm_obj_dbg (device,
+                        "removing modem%d",
+                        mm_base_modem_get_dbus_id (ctx->modem));
+            mm_device_remove_modem (device,
+                                    (GAsyncReadyCallback)modem_remove_ready,
+                                    task);
+            return;
+        }
+        ctx->step++;
+        /* fall through */
+
+    case DEVICE_SHUTDOWN_STEP_LAST:
+        mm_obj_dbg (device, "device shutdown complete");
+        g_task_return_boolean (task, TRUE);
+        g_object_unref (task);
         return;
+
+    default:
+        break;
     }
 
-    if (ctx->remove)
-        remove_device_after_disable (modem, ctx);
-    else
-        disable_context_free (ctx);
+    g_assert_not_reached ();
+}
+
+typedef struct {
+    MMBaseManager  *self;
+    GSList         *tasks;
+    GSList         *devices_to_remove;
+    MMSleepContext *sleep_ctx;
+} ManagerShutdownContext;
+
+static ManagerShutdownContext *
+manager_shutdown_context_new (MMBaseManager  *self,
+                              MMSleepContext *sleep_ctx)
+{
+    ManagerShutdownContext *ctx;
+
+    ctx = g_slice_new0 (ManagerShutdownContext);
+    ctx->self = g_object_ref (self);
+    ctx->sleep_ctx = g_object_ref (sleep_ctx);
+    return ctx;
 }
 
 static void
-foreach_disable (gpointer        key,
-                 MMDevice       *device,
-                 DisableContext *foreach_ctx)
+manager_shutdown_context_maybe_complete (ManagerShutdownContext *ctx,
+                                         GTask                  *task,
+                                         MMDevice               *device,
+                                         gboolean                remove)
 {
-    MMBaseModem    *modem;
-    DisableContext *ctx;
+    if (task)
+        ctx->tasks = g_slist_remove (ctx->tasks, task);
 
-    modem = mm_device_peek_modem (device);
-    if (!modem)
-        return;
+    if (remove) {
+        g_assert (device);
+        ctx->devices_to_remove = g_slist_append (ctx->devices_to_remove, device);
+    }
 
-    ctx = g_slice_new0 (DisableContext);
-    ctx->self = g_object_ref (foreach_ctx->self);
-    ctx->low_power = foreach_ctx->low_power;
-    ctx->remove = foreach_ctx->remove;
+    if (ctx->tasks == NULL) {
+        GSList *iter;
 
-    mm_base_modem_disable (modem,
-                           MM_BASE_MODEM_OPERATION_LOCK_REQUIRED,
-                           MM_BASE_MODEM_OPERATION_PRIORITY_OVERRIDE,
-                           (GAsyncReadyCallback)shutdown_disable_ready,
-                           ctx);
+        /* Remove devices at the end to avoid modifying the hash table
+         * during iteration, in case a device has no modem and the steps
+         * all fall through.
+         */
+        for (iter = ctx->devices_to_remove; iter; iter = iter->next)
+            g_hash_table_remove (ctx->self->priv->devices, mm_device_get_uid (device));
+        g_slist_free (ctx->devices_to_remove);
+
+        mm_obj_dbg (ctx->self, "shutdown complete");
+        mm_sleep_context_complete (ctx->sleep_ctx, NULL);
+
+        g_object_unref (ctx->sleep_ctx);
+        g_object_unref (ctx->self);
+        g_slice_free (ManagerShutdownContext, ctx);
+    }
 }
 
-static gboolean
-foreach_remove (gpointer       key,
-                MMDevice      *device,
-                MMBaseManager *self)
+static void
+device_shutdown_ready (MMDevice               *device,
+                       GAsyncResult           *res,
+                       ManagerShutdownContext *ctx)
 {
-    MMBaseModem *modem;
+    GTask                 *task;
+    DeviceShutdownContext *dctx;
+    g_autoptr(GError)      error = NULL;
 
-    modem = mm_device_peek_modem (device);
-    if (modem)
-        g_cancellable_cancel (mm_base_modem_peek_cancellable (modem));
-    mm_device_remove_modem (device);
-    return TRUE;
+    task = G_TASK (res);
+
+    if (!g_task_propagate_boolean (task, &error)) {
+        mm_obj_info (ctx->self,
+                     "failed to shutdown device %s: %s",
+                     mm_device_get_uid (device),
+                     error->message);
+    }
+
+    dctx = g_task_get_task_data (task);
+    manager_shutdown_context_maybe_complete (ctx,
+                                             task,
+                                             device,
+                                             dctx->remove);
 }
 
 void
-mm_base_manager_shutdown (MMBaseManager *self,
-                          gboolean       disable,
-                          gboolean       low_power,
-                          gboolean       remove)
+mm_base_manager_shutdown (MMBaseManager  *self,
+                          gboolean        disable,
+                          gboolean        low_power,
+                          gboolean        remove,
+                          MMSleepContext *sleep_ctx)
 {
+    ManagerShutdownContext *ctx;
+    GHashTableIter          iter;
+    MMDevice               *device;
+
     g_return_if_fail (self != NULL);
     g_return_if_fail (MM_IS_BASE_MANAGER (self));
 
     /* Cancel all ongoing auth requests */
     g_cancellable_cancel (self->priv->authp_cancellable);
 
-    if (disable) {
-        DisableContext foreach_ctx = {
-            .self = self,
-            .low_power = low_power,
-            .remove = remove,
-        };
-        g_hash_table_foreach (self->priv->devices, (GHFunc)foreach_disable, &foreach_ctx);
-
-        /* Disabling may take a few iterations of the mainloop, so the caller
-         * has to iterate the mainloop until all devices have been disabled and
-         * removed.
-         */
-        return;
-    }
+    mm_obj_dbg (self,
+                "manager shutting down... (%s%s%s%s%s)",
+                disable ? "disable" : "",
+                disable && (low_power || remove) ? "," : "",
+                low_power ? "low-power" : "",
+                (low_power && remove) ? "," : "",
+                remove ? "remove" : "");
 
-    if (remove) {
-        /* Otherwise, just remove directly */
-        g_hash_table_foreach_remove (self->priv->devices, (GHRFunc)foreach_remove, self);
+    ctx = manager_shutdown_context_new (self, sleep_ctx);
+
+    g_hash_table_iter_init (&iter, self->priv->devices);
+    while (g_hash_table_iter_next (&iter, NULL, (gpointer) &device)) {
+        DeviceShutdownContext *dctx;
+        GTask                 *task;
+
+        task = g_task_new (device,
+                           NULL,
+                           (GAsyncReadyCallback)device_shutdown_ready,
+                           ctx);
+        ctx->tasks = g_slist_append (ctx->tasks, task);
+
+        dctx = device_shutdown_context_new (mm_device_peek_modem (device),
+                                            disable,
+                                            low_power,
+                                            remove);
+        g_task_set_task_data (task, dctx, (GDestroyNotify)device_shutdown_context_free);
+
+        device_shutdown_step (task);
     }
+
+    /* Complete shutdown if there were no devices */
+    manager_shutdown_context_maybe_complete (ctx, NULL, NULL, FALSE);
 }
 
 guint32
@@ -1713,7 +1868,7 @@ handle_set_profile (MmGdbusTest *skeleton,
 out:
 
     if (error) {
-        mm_device_remove_modem (device);
+        mm_device_remove_modem_quick (device);
         g_hash_table_remove (self->priv->devices, mm_device_get_uid (device));
         mm_dbus_method_invocation_return_gerror (invocation, error);
         g_error_free (error);