Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

[Aleksander Morgado <aleksander lanedo com>]

On 30/11/12 13:32, Aleksander Morgado wrote:
>  * Whenever we wait for an unsolicited message to keep on processing an
> operation, we also need to consider the case where the port is forced to
> be closed due to being removed. At least *Icera* and *HSO* are affected
> by this issue.

Ben, are you able to retest with the attached patch applied?

Cheers,

-- 
Aleksander

>From 5b4d36e95e03b42cab2c1d7a022914f24eb9ba8b Mon Sep 17 00:00:00 2001
From: Aleksander Morgado <aleksander lanedo com>
Date: Fri, 30 Nov 2012 14:03:31 +0100
Subject: [PATCH] icera: don't wait to get connected if the primary port is
 removed

If the primary port is gone (e.g. when going to sleep) and we are just in the
middle of a connection attempt, we won't be able to receive any unsolicited
message regarding the status of the attempt. So, if we detect that the port is
forced to get closed, we'll just treat it as a connection failure.

http://code.google.com/p/chromium-os/issues/detail?id=35391
---
 plugins/icera/mm-broadband-bearer-icera.c | 30 +++++++++++++++++++++++++++++-
 src/mm-serial-port.c                      | 13 +++++++++++++
 src/mm-serial-port.h                      |  1 +
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/plugins/icera/mm-broadband-bearer-icera.c b/plugins/icera/mm-broadband-bearer-icera.c
index 7dce685..589bdc4 100644
--- a/plugins/icera/mm-broadband-bearer-icera.c
+++ b/plugins/icera/mm-broadband-bearer-icera.c
@@ -50,6 +50,7 @@ struct _MMBroadbandBearerIceraPrivate {
     gpointer connect_pending;
     guint connect_pending_id;
     gulong connect_cancellable_id;
+    gulong connect_port_closed_id;
 
     /* Disconnection related */
     gpointer disconnect_pending;
@@ -624,6 +625,12 @@ connect_timed_out_cb (MMBroadbandBearerIcera *self)
         self->priv->connect_cancellable_id = 0;
     }
 
+    /* Remove closed port watch, if found */
+    if (ctx && self->priv->connect_port_closed_id) {
+        g_signal_handler_disconnect (ctx->primary, self->priv->connect_port_closed_id);
+        self->priv->connect_port_closed_id = 0;
+    }
+
     /* Cleanup timeout ID */
     self->priv->connect_pending_id = 0;
 
@@ -665,6 +672,16 @@ connect_cancelled_cb (GCancellable *cancellable,
 }
 
 static void
+forced_close_cb (MMSerialPort *port,
+                 MMBroadbandBearerIcera *self)
+{
+    /* Just treat the forced close event as any other unsolicited message */
+    mm_broadband_bearer_icera_report_connection_status (
+        self,
+        MM_BROADBAND_BEARER_ICERA_CONNECTION_STATUS_CONNECTION_FAILED);
+}
+
+static void
 ier_query_ready (MMBaseModem *modem,
                  GAsyncResult *res,
                  Dial3gppContext *ctx)
@@ -708,7 +725,7 @@ report_connect_status (MMBroadbandBearerIcera *self,
     ctx = self->priv->connect_pending;
     self->priv->connect_pending = NULL;
 
-    /* Cleanup cancellable and timeout, if any */
+    /* Cleanup cancellable, timeout and port closed watch, if any */
     if (self->priv->connect_pending_id) {
         g_source_remove (self->priv->connect_pending_id);
         self->priv->connect_pending_id = 0;
@@ -720,6 +737,11 @@ report_connect_status (MMBroadbandBearerIcera *self,
         self->priv->connect_cancellable_id = 0;
     }
 
+    if (ctx && self->priv->connect_port_closed_id) {
+        g_signal_handler_disconnect (ctx->primary, self->priv->connect_port_closed_id);
+        self->priv->connect_port_closed_id = 0;
+    }
+
     switch (status) {
     case MM_BROADBAND_BEARER_ICERA_CONNECTION_STATUS_UNKNOWN:
         break;
@@ -845,6 +867,12 @@ activate_ready (MMBaseModem *modem,
                                                                 G_CALLBACK (connect_cancelled_cb),
                                                                 self,
                                                                 NULL);
+
+    /* If we get the port closed, we treat as a connect error */
+    self->priv->connect_port_closed_id = g_signal_connect (ctx->primary,
+                                                           "forced-close",
+                                                           G_CALLBACK (forced_close_cb),
+                                                           self);
 }
 
 static void
diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
index ce2dfeb..a02eb94 100644
--- a/src/mm-serial-port.c
+++ b/src/mm-serial-port.c
@@ -57,6 +57,7 @@ enum {
 enum {
     BUFFER_FULL,
     TIMED_OUT,
+    FORCED_CLOSE,
 
     LAST_SIGNAL
 };
@@ -1096,6 +1097,9 @@ mm_serial_port_close_force (MMSerialPort *self)
     /* Mark as having forced the close, so that we don't warn about incorrect
      * open counts */
     priv->forced_close = TRUE;
+
+    /* Notify about the forced close status */
+    g_signal_emit (self, signals[FORCED_CLOSE], 0);
 }
 
 static void
@@ -1673,4 +1677,13 @@ mm_serial_port_class_init (MMSerialPortClass *klass)
                       NULL, NULL,
                       g_cclosure_marshal_VOID__UINT,
 					  G_TYPE_NONE, 1, G_TYPE_UINT);
+
+    signals[FORCED_CLOSE] =
+        g_signal_new ("forced-close",
+                  G_OBJECT_CLASS_TYPE (object_class),
+                  G_SIGNAL_RUN_FIRST,
+                  G_STRUCT_OFFSET (MMSerialPortClass, forced_close),
+                  NULL, NULL,
+                  g_cclosure_marshal_VOID__VOID,
+                  G_TYPE_NONE, 0);
 }
diff --git a/src/mm-serial-port.h b/src/mm-serial-port.h
index 77db321..31cd5a7 100644
--- a/src/mm-serial-port.h
+++ b/src/mm-serial-port.h
@@ -100,6 +100,7 @@ struct _MMSerialPortClass {
     /* Signals */
     void (*buffer_full)           (MMSerialPort *port, const GByteArray *buffer);
     void (*timed_out)             (MMSerialPort *port, guint n_consecutive_replies);
+    void (*forced_close)          (MMSerialPort *port);
 };
 
 GType mm_serial_port_get_type (void);
-- 
1.8.0