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, 177 insertions, 16 deletions

diff --git a/src/mm-base-modem.c b/src/mm-base-modem.c
index 15469b5c..7e142327 100644
--- a/src/mm-base-modem.c
+++ b/src/mm-base-modem.c
@@ -87,6 +87,7 @@ struct _MMBaseModemPrivate {
      * should be done by the modem. */
     GCancellable *cancellable;
     gulong invalid_if_cancelled;
+    guint  invalid_from_idle;
 
     gchar *device;
     gchar *physdev;
@@ -101,6 +102,7 @@ struct _MMBaseModemPrivate {
     gboolean hotplugged;
     gboolean valid;
     gboolean reprobe;
+    gboolean torn_down;
 
     guint max_timeouts;
 
@@ -2161,13 +2163,22 @@ mm_base_modem_get_subsystem_device_id (MMBaseModem *self)
 
 /*****************************************************************************/
 
+static void
+clear_invalid_from_idle (MMBaseModem *self)
+{
+    if (self->priv->invalid_from_idle)
+        g_source_remove (self->priv->invalid_from_idle);
+    self->priv->invalid_from_idle = 0;
+}
+
 static gboolean
 base_modem_invalid_idle (MMBaseModem *self)
 {
+    clear_invalid_from_idle (self);
+
     /* Ensure the modem is set invalid if we get the modem-wide cancellable
      * cancelled */
     mm_base_modem_set_valid (self, FALSE);
-    g_object_unref (self);
     return G_SOURCE_REMOVE;
 }
 
@@ -2175,9 +2186,14 @@ static void
 base_modem_cancelled (GCancellable *cancellable,
                       MMBaseModem *self)
 {
+    clear_invalid_from_idle (self);
+
     /* NOTE: Don't call set_valid() directly here, do it in an idle, and ensure
      * that we pass a valid reference of the modem object as context. */
-    g_idle_add ((GSourceFunc)base_modem_invalid_idle, g_object_ref (self));
+    self->priv->invalid_from_idle = g_idle_add_full (G_PRIORITY_DEFAULT_IDLE,
+                                                     (GSourceFunc)base_modem_invalid_idle,
+                                                     g_object_ref (self),
+                                                     (GDestroyNotify) g_object_unref);
 }
 
 /*****************************************************************************/
@@ -2189,9 +2205,107 @@ setup_ports_table (GHashTable **ht)
     *ht = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_object_unref);
 }
 
+typedef struct {
+    volatile gint  refcount;
+    GError        *error;
+    /* The Context owns the GTask so that we can ensure the Task returns only
+     * when all ports are closed. The reference count tracks open ports and
+     * the context will return the Task when it reaches zero (indicating all
+     * outstanding operations are complete and ports are closed).
+     */
+    GTask         *task;
+} TeardownContext;
+
+static TeardownContext *
+teardown_context_new (GObject             *source_object,
+                      GAsyncReadyCallback  callback,
+                      gpointer             user_data)
+{
+    TeardownContext *ctx;
+
+    ctx = g_slice_new0 (TeardownContext);
+    ctx->refcount = 1;
+    ctx->task = g_task_new (source_object, NULL, callback, user_data);
+    return ctx;
+}
+
+static void
+teardown_context_ref (TeardownContext *ctx)
+{
+    g_atomic_int_inc (&ctx->refcount);
+}
+
+static void
+teardown_context_unref (TeardownContext *ctx)
+{
+    if (g_atomic_int_dec_and_test (&ctx->refcount)) {
+        if (ctx->error)
+            g_task_return_error (ctx->task, g_steal_pointer (&ctx->error));
+        else
+            g_task_return_boolean (ctx->task, TRUE);
+        g_clear_object (&ctx->task);
+        g_slice_free (TeardownContext, ctx);
+    }
+}
+
+#if defined (WITH_MBIM) || defined (WITH_QMI)
+
+static void
+teardown_context_add_error (TeardownContext *ctx,
+                            MMPort          *port,
+                            GError          *error)
+{
+    if (!ctx->error) {
+        ctx->error = g_error_copy (error);
+        g_prefix_error (&ctx->error,
+                        "[%s] teardown error: ",
+                        mm_port_get_device (port));
+    } else {
+        g_prefix_error (&ctx->error,
+                        "[%s] teardown error: %s; ",
+                        mm_port_get_device (port),
+                        error->message);
+    }
+}
+
+#endif
+
+#if defined WITH_MBIM
+
+static void
+mbim_port_close_ready (MMPortMbim      *port,
+                       GAsyncResult    *res,
+                       TeardownContext *ctx)
+{
+    g_autoptr(GError) error = NULL;
+
+    if (!mm_port_mbim_close_finish (port, res, &error))
+        teardown_context_add_error (ctx, MM_PORT (port), error);
+    teardown_context_unref (ctx);
+}
+
+#endif
+
+#if defined WITH_QMI
+
+static void
+qmi_port_close_ready (MMPortQmi       *port,
+                      GAsyncResult    *res,
+                      TeardownContext *ctx)
+{
+    g_autoptr(GError) error = NULL;
+
+    if (!mm_port_qmi_close_finish (port, res, &error))
+        teardown_context_add_error (ctx, MM_PORT (port), error);
+    teardown_context_unref (ctx);
+}
+
+#endif
+
 static void
-cleanup_modem_port (MMBaseModem *self,
-                    MMPort      *port)
+cleanup_modem_port (MMBaseModem     *self,
+                    MMPort          *port,
+                    TeardownContext *ctx)
 {
     mm_obj_dbg (self, "cleaning up port '%s/%s'...",
                 mm_port_subsys_get_string (mm_port_get_subsys (MM_PORT (port))),
@@ -2201,10 +2315,20 @@ cleanup_modem_port (MMBaseModem *self,
     g_signal_handlers_disconnect_by_func (port, port_timed_out_cb, self);
     g_signal_handlers_disconnect_by_func (port, port_removed_cb, self);
 
+    if (ctx)
+        teardown_context_ref (ctx);
+
+    /* No need to close serial ports here as they do not require a specific
+     * shutdown procedure with message exchanges and callbacks. They will be
+     * closed when the modem is invalidated or disposed.
+     */
+
 #if defined WITH_MBIM
     /* We need to close the MBIM port cleanly when disposing the modem object */
     if (MM_IS_PORT_MBIM (port)) {
-        mm_port_mbim_close (MM_PORT_MBIM (port), NULL, NULL);
+        mm_port_mbim_close (MM_PORT_MBIM (port),
+                            ctx ? (GAsyncReadyCallback)mbim_port_close_ready : NULL,
+                            ctx);
         return;
     }
 #endif
@@ -2215,27 +2339,61 @@ cleanup_modem_port (MMBaseModem *self,
      * allocating too many newer allocations will fail with client-ids-exhausted
      * errors. */
     if (MM_IS_PORT_QMI (port)) {
-        mm_port_qmi_close (MM_PORT_QMI (port), NULL, NULL);
+        mm_port_qmi_close (MM_PORT_QMI (port),
+                           ctx ? (GAsyncReadyCallback)qmi_port_close_ready : NULL,
+                           ctx);
         return;
     }
 #endif
+
+    if (ctx)
+        teardown_context_unref (ctx);
 }
 
 static void
-teardown_ports_table (MMBaseModem  *self,
-                      GHashTable  **ht)
+teardown_ports_tables (MMBaseModem     *self,
+                       TeardownContext *ctx)
 {
+    GHashTable **tables[2] = {
+        &self->priv->link_ports,
+        &self->priv->ports,
+    };
     GHashTableIter iter;
     gpointer       value;
     gpointer       key;
+    guint          i;
+
+    for (i = 0; i < G_N_ELEMENTS (tables); i++) {
+        if (*tables[i]) {
+            g_hash_table_iter_init (&iter, *tables[i]);
+            while (g_hash_table_iter_next (&iter, &key, &value))
+                cleanup_modem_port (self, MM_PORT (value), ctx);
+            g_hash_table_destroy (*tables[i]);
+            *tables[i] = NULL;
+        }
+    }
+}
 
-    if (!*ht)
-        return;
+gboolean
+mm_base_modem_teardown_ports_finish (MMBaseModem   *self,
+                                     GAsyncResult  *res,
+                                     GError       **error)
+{
+    return g_task_propagate_boolean (G_TASK (res), error);
+}
+
+void
+mm_base_modem_teardown_ports (MMBaseModem         *self,
+                              GAsyncReadyCallback  callback,
+                              gpointer             user_data)
+{
+    TeardownContext *ctx;
 
-    g_hash_table_iter_init (&iter, *ht);
-    while (g_hash_table_iter_next (&iter, &key, &value))
-        cleanup_modem_port (self, MM_PORT (value));
-    g_hash_table_destroy (g_steal_pointer (ht));
+    self->priv->torn_down = TRUE;
+
+    ctx = teardown_context_new (G_OBJECT (self), callback, user_data);
+    teardown_ports_tables (self, ctx);
+    teardown_context_unref (ctx);
 }
 
 /*****************************************************************************/
@@ -2441,6 +2599,8 @@ dispose (GObject *object)
     g_cancellable_cancel (self->priv->cancellable);
     g_clear_object (&self->priv->cancellable);
 
+    clear_invalid_from_idle (self);
+
     g_clear_object (&self->priv->primary);
     g_clear_object (&self->priv->secondary);
     g_list_free_full (g_steal_pointer (&self->priv->data), g_object_unref);
@@ -2455,8 +2615,9 @@ dispose (GObject *object)
     g_list_free_full (g_steal_pointer (&self->priv->mbim), g_object_unref);
 #endif
 
-    teardown_ports_table (self, &self->priv->link_ports);
-    teardown_ports_table (self, &self->priv->ports);
+    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);