port-qmi: implement the new generic 'removed' signal

Letting the MMBroadbandModemQmi listen for the notifications of the
QmiDevice was not a good idea, especially since no explicit reference
to the device object was hold in the modem object. This leads to race
conditions in which the QmiDevice outlives the MMBroadbandModemQmi and
the MMPortQmi, and when the "device-removed" signal is triggered, the
program crashes.

The MMPortQmi will now emit its own 'removed' signals when the
underlying QmiDevice emits "device-removed', ensuring that the signal
handler is properly cleared up during the MMPortQmi disposal.

3 files changed, 27 insertions, 97 deletions

diff --git a/src/mm-broadband-modem-qmi.c b/src/mm-broadband-modem-qmi.c
index 98e53bba3..284564d1f 100644
--- a/src/mm-broadband-modem-qmi.c
+++ b/src/mm-broadband-modem-qmi.c
@@ -12873,60 +12873,6 @@ parent_initialization_started (GTask *task)
         task);
 }
 
-static void
-qmi_device_removed_cb (QmiDevice *device,
-                       MMBroadbandModemQmi *self)
-{
-    /* Reprobe the modem here so we can get notifications back. */
-    mm_obj_msg (self, "connection to qmi-proxy for %s lost, reprobing",
-                qmi_device_get_path_display (device));
-
-    g_signal_handler_disconnect (device, self->priv->qmi_device_removed_id);
-    self->priv->qmi_device_removed_id = 0;
-
-    mm_base_modem_set_reprobe (MM_BASE_MODEM (self), TRUE);
-    mm_base_modem_set_valid (MM_BASE_MODEM (self), FALSE);
-}
-
-static gboolean
-track_qmi_device_removed (MMBroadbandModemQmi  *self,
-                          MMPortQmi            *qmi,
-                          GError              **error)
-{
-    QmiDevice *device;
-
-    device = mm_port_qmi_peek_device (qmi);
-    if (!device) {
-        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
-                     "Cannot track QMI device removal: QMI port no longer available");
-        return FALSE;
-    }
-
-    self->priv->qmi_device_removed_id = g_signal_connect (
-        device,
-        QMI_DEVICE_SIGNAL_REMOVED,
-        G_CALLBACK (qmi_device_removed_cb),
-        self);
-    return TRUE;
-}
-
-static void
-untrack_qmi_device_removed (MMBroadbandModemQmi *self,
-                            MMPortQmi           *qmi)
-{
-    QmiDevice *device;
-
-    if (self->priv->qmi_device_removed_id == 0)
-        return;
-
-    device = mm_port_qmi_peek_device (qmi);
-    if (!device)
-        return;
-
-    g_signal_handler_disconnect (device, self->priv->qmi_device_removed_id);
-    self->priv->qmi_device_removed_id = 0;
-}
-
 static void allocate_next_client (GTask *task);
 
 static void
@@ -12956,20 +12902,10 @@ static void
 allocate_next_client (GTask *task)
 {
     InitializationStartedContext *ctx;
-    MMBroadbandModemQmi          *self;
 
-    self = g_task_get_source_object (task);
     ctx = g_task_get_task_data (task);
 
     if (ctx->service_index == G_N_ELEMENTS (qmi_services)) {
-        GError *error = NULL;
-
-        /* Done we are, track device removal and launch next step */
-        if (!track_qmi_device_removed (self, ctx->qmi, &error)) {
-            g_task_return_error (task, error);
-            g_object_unref (task);
-            return;
-        }
         parent_initialization_started (task);
         return;
     }
@@ -13086,7 +13022,6 @@ initialization_started (MMBroadbandModem *self,
 {
     InitializationStartedContext *ctx;
     GTask                        *task;
-    GError                       *error = NULL;
 
     ctx = g_new0 (InitializationStartedContext, 1);
     ctx->qmi = mm_broadband_modem_qmi_get_port_qmi (MM_BROADBAND_MODEM_QMI (self));
@@ -13105,14 +13040,6 @@ initialization_started (MMBroadbandModem *self,
     }
 
     if (mm_port_qmi_is_open (ctx->qmi)) {
-        /* Nothing to be done, just track device removal and launch parent's
-         * callback */
-        if (!track_qmi_device_removed (MM_BROADBAND_MODEM_QMI (self), ctx->qmi, &error)) {
-            g_task_return_error (task, error);
-            g_object_unref (task);
-            return;
-        }
-
         parent_initialization_started (task);
         return;
     }
@@ -13183,8 +13110,6 @@ dispose (GObject *object)
      * that will remove all port references right away */
     qmi = mm_broadband_modem_qmi_peek_port_qmi (self);
     if (qmi) {
-        /* Disconnect signal handler for qmi-proxy disappearing, if it exists */
-        untrack_qmi_device_removed (self, qmi);
         /* If we did open the QMI port during initialization, close it now */
         if (mm_port_qmi_is_open (qmi))
             mm_port_qmi_close (qmi, NULL, NULL);
diff --git a/src/mm-port-qmi.c b/src/mm-port-qmi.c
index 41f6033d5..aec1c0188 100644
--- a/src/mm-port-qmi.c
+++ b/src/mm-port-qmi.c
@@ -64,8 +64,9 @@ struct _MMPortQmiPrivate {
     QrtrNode  *node;
 #endif
 
-    /* timeout monitoring */
+    /* port monitoring */
     gulong timeout_monitoring_id;
+    gulong removed_monitoring_id;
     /* endpoint info */
     QmiDataEndpointType endpoint_type;
     gint                endpoint_interface_number;
@@ -131,16 +132,6 @@ mm_port_qmi_get_client (MMPortQmi  *self,
 
 /*****************************************************************************/
 
-QmiDevice *
-mm_port_qmi_peek_device (MMPortQmi *self)
-{
-    g_return_val_if_fail (MM_IS_PORT_QMI (self), NULL);
-
-    return self->priv->qmi_device;
-}
-
-/*****************************************************************************/
-
 static void
 initialize_endpoint_info (MMPortQmi *self)
 {
@@ -196,13 +187,17 @@ mm_port_qmi_get_endpoint_info (MMPortQmi *self, MMQmiDataEndpoint *out_endpoint)
 /*****************************************************************************/
 
 static void
-reset_timeout_monitoring (MMPortQmi *self,
-                          QmiDevice *qmi_device)
+reset_monitoring (MMPortQmi *self,
+                  QmiDevice *qmi_device)
 {
     if (self->priv->timeout_monitoring_id && qmi_device) {
         g_signal_handler_disconnect (qmi_device, self->priv->timeout_monitoring_id);
         self->priv->timeout_monitoring_id = 0;
     }
+    if (self->priv->removed_monitoring_id && qmi_device) {
+        g_signal_handler_disconnect (qmi_device, self->priv->removed_monitoring_id);
+        self->priv->removed_monitoring_id = 0;
+    }
 }
 
 static void
@@ -214,18 +209,30 @@ consecutive_timeouts_updated_cb (MMPortQmi  *self,
 }
 
 static void
-setup_timeout_monitoring (MMPortQmi *self,
-                          QmiDevice *qmi_device)
+device_removed_cb (MMPortQmi  *self)
+{
+    g_signal_emit_by_name (self, MM_PORT_SIGNAL_REMOVED);
+}
+
+static void
+setup_monitoring (MMPortQmi *self,
+                  QmiDevice *qmi_device)
 {
     g_assert (qmi_device);
 
-    reset_timeout_monitoring (self, qmi_device);
+    reset_monitoring (self, qmi_device);
 
     g_assert (!self->priv->timeout_monitoring_id);
     self->priv->timeout_monitoring_id = g_signal_connect_swapped (qmi_device,
                                                                   "notify::" QMI_DEVICE_CONSECUTIVE_TIMEOUTS,
                                                                   G_CALLBACK (consecutive_timeouts_updated_cb),
                                                                   self);
+
+    g_assert (!self->priv->removed_monitoring_id);
+    self->priv->removed_monitoring_id = g_signal_connect_swapped (qmi_device,
+                                                                  QMI_DEVICE_SIGNAL_REMOVED,
+                                                                  G_CALLBACK (device_removed_cb),
+                                                                  self);
 }
 
 /*****************************************************************************/
@@ -2532,7 +2539,7 @@ port_open_step (GTask *task)
         g_assert (ctx->device);
         g_assert (!self->priv->qmi_device);
         self->priv->qmi_device = g_object_ref (ctx->device);
-        setup_timeout_monitoring (self, ctx->device);
+        setup_monitoring (self, ctx->device);
         self->priv->in_progress = FALSE;
         g_task_return_boolean (task, TRUE);
         g_object_unref (task);
@@ -2672,8 +2679,8 @@ mm_port_qmi_close (MMPortQmi           *self,
     ctx->qmi_device = g_steal_pointer (&self->priv->qmi_device);
     g_task_set_task_data (task, ctx, (GDestroyNotify)port_qmi_close_context_free);
 
-    /* Reset timeout monitoring logic */
-    reset_timeout_monitoring (self, ctx->qmi_device);
+    /* Reset monitoring logic */
+    reset_monitoring (self, ctx->qmi_device);
 
     /* Release all allocated clients */
     for (l = self->priv->services; l; l = g_list_next (l)) {
@@ -2804,7 +2811,7 @@ dispose (GObject *object)
     g_clear_object (&self->priv->node);
 #endif
     /* Clear device object */
-    reset_timeout_monitoring (self, self->priv->qmi_device);
+    reset_monitoring (self, self->priv->qmi_device);
     g_clear_object (&self->priv->qmi_device);
 
     g_clear_pointer (&self->priv->net_driver, g_free);
diff --git a/src/mm-port-qmi.h b/src/mm-port-qmi.h
index 7b434cdb5..854b52455 100644
--- a/src/mm-port-qmi.h
+++ b/src/mm-port-qmi.h
@@ -121,8 +121,6 @@ QmiClient *mm_port_qmi_get_client  (MMPortQmi  *self,
                                     QmiService  service,
                                     guint       flag);
 
-QmiDevice *mm_port_qmi_peek_device (MMPortQmi *self);
-
 typedef struct {
     QmiDataEndpointType type;
     guint               interface_number;