From 41fce3efcc272e21e14dc7d9441c7ba64ca6fba4 Mon Sep 17 00:00:00 2001
From: Dan Williams <dan@ioncontrol.co>
Date: Sat, 8 Mar 2025 20:00:13 -0600
Subject: [PATCH] device,base-manager: make mm_device_remove_modem() async

Signed-off-by: Dan Williams <dan@ioncontrol.co>
---
 src/mm-base-manager.c | 124 ++++++++++++++++++++++++------------------
 src/mm-base-modem.c   |   4 ++
 src/mm-device.c       |  61 ++++++++++++++++++++-
 src/mm-device.h       |  14 +++--
 4 files changed, 143 insertions(+), 60 deletions(-)

diff --git a/src/mm-base-manager.c b/src/mm-base-manager.c
index 0f414dc25..7ea77c120 100644
--- a/src/mm-base-manager.c
+++ b/src/mm-base-manager.c
@@ -367,7 +367,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));
     }
 }
@@ -501,7 +501,7 @@ 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);
+    mm_device_remove_modem_quick (device);
     device_support_check_add_all_ports (self, device);
 }
 
@@ -881,17 +881,17 @@ typedef struct {
     gboolean        low_power;
     gboolean        remove;
     MMSleepContext *sleep_ctx;
-} DisableContext;
+} ShutdownContext;
 
-static DisableContext *
-disable_context_new (MMBaseManager  *self,
+static ShutdownContext *
+shutdown_context_new (MMBaseManager *self,
                      gboolean        low_power,
                      gboolean        remove,
                      MMSleepContext *sleep_ctx)
 {
-    DisableContext *ctx;
+    ShutdownContext *ctx;
 
-    ctx = g_slice_new0 (DisableContext);
+    ctx = g_slice_new0 (ShutdownContext);
     ctx->refcount = 1;
     ctx->self = g_object_ref (self);
     ctx->low_power = low_power;
@@ -901,7 +901,7 @@ disable_context_new (MMBaseManager  *self,
 }
 
 static void
-disable_context_unref (DisableContext *ctx)
+shutdown_context_unref (ShutdownContext *ctx)
 {
     ctx->refcount--;
     if (ctx->refcount == 0) {
@@ -909,40 +909,58 @@ g_message ("###### [%p] all done, completing", ctx->sleep_ctx);
         mm_sleep_context_complete (ctx->sleep_ctx, NULL);
         g_object_unref (ctx->sleep_ctx);
         g_object_unref (ctx->self);
-        g_slice_free (DisableContext, ctx);
+        g_slice_free (ShutdownContext, ctx);
     }
 }
 
 static void
-disable_context_ref (DisableContext *ctx)
+shutdown_context_ref (ShutdownContext *ctx)
 {
     ctx->refcount++;
 }
 
 static void
-remove_device_after_disable (MMBaseModem    *modem,
-                             DisableContext *ctx)
+modem_remove_ready (MMDevice        *device,
+                    GAsyncResult    *res,
+                    ShutdownContext *ctx)
+{
+    g_autoptr(GError) error = NULL;
+
+g_message ("###### %s: [%p] remove done", __func__, ctx->sleep_ctx);
+    if (!mm_device_remove_modem_finish (device, res, &error))
+        mm_obj_warn (ctx->self, "removing modem failed: %s", error->message);
+    g_hash_table_remove (ctx->self->priv->devices, mm_device_get_uid (device));
+    /* balance foreach_remove() and remove_device_after_disable() */
+    g_object_unref (device);
+g_message ("###### %s: [%p] all done", __func__, ctx->sleep_ctx);
+    shutdown_context_unref (ctx);
+}
+
+static void
+remove_device_after_disable (MMBaseModem     *modem,
+                             ShutdownContext *ctx)
 {
     MMDevice *device;
-    gchar *name = g_strdup (mm_base_modem_get_device (modem));
 
-g_message ("###### [%p] base manager remove ready %s", ctx->sleep_ctx, mm_base_modem_get_device (MM_BASE_MODEM (modem)));
+g_message ("###### %s: [%p] about to remove %s", __func__, ctx->sleep_ctx, mm_base_modem_get_device (MM_BASE_MODEM (modem)));
     device = find_device_by_modem (ctx->self, modem);
     if (device) {
+        /* Keep the device alive over the removal */
+        g_object_ref (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));
+        mm_device_remove_modem (device,
+                                (GAsyncReadyCallback)modem_remove_ready,
+                                ctx);
+        return;
     }
 
-g_message ("###### [%p] base manager remove done %s", ctx->sleep_ctx, name);
-g_free (name);
-    disable_context_unref (ctx);
+    shutdown_context_unref (ctx);
 }
 
 static void
-shutdown_low_power_ready (MMIfaceModem   *modem,
-                          GAsyncResult   *res,
-                          DisableContext *ctx)
+shutdown_low_power_ready (MMIfaceModem    *modem,
+                          GAsyncResult    *res,
+                          ShutdownContext *ctx)
 {
     g_autoptr(GError) error = NULL;
 
@@ -957,13 +975,13 @@ g_message ("###### [%p] base manager remove %s", ctx->sleep_ctx, mm_base_modem_g
     }
 
 g_message ("###### [%p] base manager low power fail %s", ctx->sleep_ctx, mm_base_modem_get_device (MM_BASE_MODEM (modem)));
-    disable_context_unref (ctx);
+    shutdown_context_unref (ctx);
 }
 
 static void
-shutdown_disable_ready (MMBaseModem    *modem,
-                        GAsyncResult   *res,
-                        DisableContext *ctx)
+shutdown_disable_ready (MMBaseModem     *modem,
+                        GAsyncResult    *res,
+                        ShutdownContext *ctx)
 {
     g_autoptr(GError) error = NULL;
 
@@ -988,13 +1006,13 @@ g_message ("###### [%p] base manager low power %s", ctx->sleep_ctx, mm_base_mode
     }
 
 g_message ("###### [%p] base manager disable fail %s", ctx->sleep_ctx, mm_base_modem_get_device (modem));
-    disable_context_unref (ctx);
+    shutdown_context_unref (ctx);
 }
 
 static void
-foreach_disable (gpointer        key,
-                 MMDevice       *device,
-                 DisableContext *ctx)
+foreach_disable (gpointer         key,
+                 MMDevice        *device,
+                 ShutdownContext *ctx)
 {
     MMBaseModem    *modem;
 
@@ -1002,7 +1020,7 @@ foreach_disable (gpointer        key,
     if (!modem)
         return;
 
-    disable_context_ref (ctx);
+    shutdown_context_ref (ctx);
 g_message ("###### [%p] base manager disabling %s ref %u", ctx->sleep_ctx, mm_base_modem_get_device (modem), ctx->refcount);
     mm_base_modem_disable (modem,
                            MM_BASE_MODEM_OPERATION_LOCK_REQUIRED,
@@ -1012,16 +1030,23 @@ g_message ("###### [%p] base manager disabling %s ref %u", ctx->sleep_ctx, mm_ba
 }
 
 static gboolean
-foreach_remove (gpointer       key,
-                MMDevice      *device,
-                MMBaseManager *self)
+foreach_remove (gpointer         key,
+                MMDevice        *device,
+                ShutdownContext *ctx)
 {
     MMBaseModem *modem;
 
+    /* Keep the device alive over the removal */
+    g_object_ref (device);
+
     modem = mm_device_peek_modem (device);
     if (modem)
         g_cancellable_cancel (mm_base_modem_peek_cancellable (modem));
-    mm_device_remove_modem (device);
+
+    shutdown_context_ref (ctx);
+    mm_device_remove_modem (device,
+                            (GAsyncReadyCallback)modem_remove_ready,
+                            ctx);
     return TRUE;
 }
 
@@ -1032,37 +1057,30 @@ mm_base_manager_shutdown (MMBaseManager  *self,
                           gboolean        remove,
                           MMSleepContext *sleep_ctx)
 {
+    ShutdownContext *ctx;
+
     g_return_if_fail (self != NULL);
     g_return_if_fail (MM_IS_BASE_MANAGER (self));
 
+    ctx = shutdown_context_new (self, low_power, remove, sleep_ctx);
+
     /* Cancel all ongoing auth requests */
     g_cancellable_cancel (self->priv->authp_cancellable);
 
 g_message ("###### [%p] base manager shutdown", sleep_ctx);
 
     if (disable) {
-        DisableContext *ctx;
-
-        ctx = disable_context_new (self, low_power, remove, sleep_ctx);
 g_message ("###### [%p] base manager disabling", sleep_ctx);
         g_hash_table_foreach (self->priv->devices, (GHFunc)foreach_disable, ctx);
-        disable_context_unref (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;
-    }
-
-    if (remove) {
-        /* Otherwise, just remove directly */
+g_message ("###### [%p] base manager disabling DONE", sleep_ctx);
+    } else if (remove) {
 g_message ("###### [%p] base manager remove", sleep_ctx);
-        g_hash_table_foreach_remove (self->priv->devices, (GHRFunc)foreach_remove, self);
+        g_hash_table_foreach_remove (self->priv->devices, (GHRFunc)foreach_remove, ctx);
 g_message ("###### [%p] base manager remove DONE", sleep_ctx);
-        mm_sleep_context_complete (sleep_ctx, NULL);
-g_message ("###### [%p] base manager ctx COMPLETE", sleep_ctx);
     }
+
+g_message ("###### [%p] base manager ctx COMPLETE", sleep_ctx);
+    shutdown_context_unref (ctx);
 }
 
 guint32
@@ -1763,7 +1781,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);
diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c
index 5de057f43..66aa7cf74 100644
--- a/src/mm-base-modem.c
+++ b/src/mm-base-modem.c
@@ -101,6 +101,7 @@ struct _MMBaseModemPrivate {
     gboolean hotplugged;
     gboolean valid;
     gboolean reprobe;
+    gboolean torn_down;
 
     guint max_timeouts;
 
@@ -2321,6 +2322,7 @@ teardown_ports_tables (MMBaseModem     *self,
 
     self->priv->link_ports = NULL;
     self->priv->ports = NULL;
+    self->priv->torn_down = TRUE;
 }
 
 gboolean
@@ -2560,6 +2562,8 @@ dispose (GObject *object)
     g_list_free_full (g_steal_pointer (&self->priv->mbim), g_object_unref);
 #endif
 
+    if (!self->priv->torn_down)
+        mm_obj_warn (self, "teardown not called before dispose");
     teardown_ports_tables (self, NULL);
 
     g_clear_object (&self->priv->connection);
diff --git a/src/mm-device.c b/src/mm-device.c
index 109ba21ea..35baca50f 100644
--- a/src/mm-device.c
+++ b/src/mm-device.c
@@ -459,8 +459,63 @@ clear_modem (MMDevice *self)
     }
 }
 
+gboolean
+mm_device_remove_modem_finish (MMDevice        *self,
+                               GAsyncResult    *res,
+                               GError         **error)
+{
+    return g_task_propagate_boolean (G_TASK (res), error);
+}
+
+static void
+teardown_ports_ready (MMBaseModem  *modem,
+                      GAsyncResult *res,
+                      GTask        *task)
+{
+    MMDevice *self;
+    GError   *error = NULL;
+    gboolean  success;
+
+    self = g_task_get_source_object (task);
+
+    success = mm_base_modem_teardown_ports_finish (self->priv->modem, res, &error);
+
+g_message ("###### %s: about to clear", __func__);
+    clear_modem (self);
+g_message ("###### %s: clear DONE", __func__);
+
+    if (!success)
+        g_task_return_error (task, error);
+    else
+        g_task_return_boolean (task, TRUE);
+
+g_message ("###### %s: remove complete", __func__);
+
+    g_object_unref (task);
+}
+
+void
+mm_device_remove_modem (MMDevice            *self,
+                        GAsyncReadyCallback  callback,
+                        gpointer             user_data)
+{
+    GTask *task;
+
+    task = g_task_new (self, NULL, callback, user_data);
+    if (!self->priv->modem) {
+        g_task_return_boolean (task, TRUE);
+        g_object_unref (task);
+        return;
+    }
+
+    unexport_modem (self);
+    mm_base_modem_teardown_ports (self->priv->modem,
+                                  (GAsyncReadyCallback)teardown_ports_ready,
+                                  task);
+}
+
 void
-mm_device_remove_modem (MMDevice  *self)
+mm_device_remove_modem_quick (MMDevice *self)
 {
     if (!self->priv->modem)
         return;
@@ -498,7 +553,7 @@ modem_valid (MMBaseModem *modem,
 {
     if (!mm_base_modem_get_valid (modem)) {
         /* Modem no longer valid */
-        mm_device_remove_modem (self);
+        mm_device_remove_modem_quick (self);
         if (mm_base_modem_get_reprobe (modem))
             self->priv->reprobe_id = g_timeout_add_seconds (REPROBE_SECS, (GSourceFunc)reprobe, self);
     } else {
@@ -746,7 +801,7 @@ inhibit_disable_ready (MMBaseModem  *modem,
         g_task_return_error (task, error);
     else {
         g_cancellable_cancel (mm_base_modem_peek_cancellable (modem));
-        mm_device_remove_modem (self);
+        mm_device_remove_modem_quick (self);
         g_task_return_boolean (task, TRUE);
     }
     g_object_unref (task);
diff --git a/src/mm-device.h b/src/mm-device.h
index f8cefc1d7..127320564 100644
--- a/src/mm-device.h
+++ b/src/mm-device.h
@@ -80,10 +80,16 @@ void     mm_device_release_port_name (MMDevice       *self,
                                       const gchar    *subsystem,
                                       const gchar    *name);
 
-gboolean mm_device_create_modem     (MMDevice  *self,
-                                     GError   **error);
-void     mm_device_remove_modem     (MMDevice  *self);
-void     mm_device_initialize_modem (MMDevice *self);
+gboolean mm_device_create_modem        (MMDevice            *self,
+                                        GError             **error);
+void     mm_device_remove_modem        (MMDevice            *self,
+                                        GAsyncReadyCallback  callback,
+                                        gpointer             user_data);
+gboolean mm_device_remove_modem_finish (MMDevice            *self,
+                                        GAsyncResult        *res,
+                                        GError             **error);
+void     mm_device_remove_modem_quick  (MMDevice            *self);
+void     mm_device_initialize_modem    (MMDevice            *self);
 
 void     mm_device_inhibit        (MMDevice                  *self,
                                    GAsyncReadyCallback        callback,
-- 
GitLab