<div dir="ltr">thanks. lgtm.  The <span style="font-size:12.8px">g_signal_handler_is_connected check in clear_modem seems unnecessary though?</span><div><span style="font-size:12.8px"><br></span><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 13, 2017 at 6:06 AM, Aleksander Morgado <span dir="ltr"><<a href="mailto:aleksander@aleksander.es" target="_blank">aleksander@aleksander.es</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Don't rely on the automatic disconnection of the signal as the last<br>
reference of the modem object may outlive the device object.<br>
<br>
Also, setup a common clear_modem() function to dispose the internal<br>
reference of the modem object, which will take care of the signal<br>
disconnection when required.<br>
<br>
<a href="https://lists.freedesktop.org/archives/modemmanager-devel/2017-February/003907.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>archives/modemmanager-devel/<wbr>2017-February/003907.html</a><br>
---<br>
<br>
What do you think of this?<br>
<br>
---<br>
 src/mm-device.c | 35 ++++++++++++++++++++++++------<wbr>-----<br>
 1 file changed, 24 insertions(+), 11 deletions(-)<br>
<br>
diff --git a/src/mm-device.c b/src/mm-device.c<br>
index 6750a0b1..20fb9578 100644<br>
--- a/src/mm-device.c<br>
+++ b/src/mm-device.c<br>
@@ -71,6 +71,7 @@ struct _MMDevicePrivate {<br>
<br>
     /* The Modem object for this device */<br>
     MMBaseModem *modem;<br>
+    gulong       modem_valid_id;<br>
<br>
     /* When exported, a reference to the object manager */<br>
     GDBusObjectManagerServer *object_manager;<br>
@@ -293,6 +294,21 @@ export_modem (MMDevice *self)<br>
<br>
 /*****************************<wbr>******************************<wbr>******************/<br>
<br>
+static void<br>
+clear_modem (MMDevice *self)<br>
+{<br>
+    if (self->priv->modem_valid_id) {<br>
+        if (self->priv->modem && g_signal_handler_is_connected (self->priv->modem, self->priv->modem_valid_id))<br>
+            g_signal_handler_disconnect (self->priv->modem, self->priv->modem_valid_id);<br>
+        self->priv->modem_valid_id = 0;<br>
+    }<br>
+<br>
+    /* Run dispose before unref-ing, in order to cleanup the SIM object,<br>
+     * if any (which also holds a reference to the modem object) */<br>
+    g_object_run_dispose (G_OBJECT (self->priv->modem));<br>
+    g_clear_object (&(self->priv->modem));<br>
+}<br>
+<br>
 void<br>
 mm_device_remove_modem (MMDevice  *self)<br>
 {<br>
@@ -300,11 +316,7 @@ mm_device_remove_modem (MMDevice  *self)<br>
         return;<br>
<br>
     unexport_modem (self);<br>
-<br>
-    /* Run dispose before unref-ing, in order to cleanup the SIM object,<br>
-     * if any (which also holds a reference to the modem object) */<br>
-    g_object_run_dispose (G_OBJECT (self->priv->modem));<br>
-    g_clear_object (&(self->priv->modem));<br>
+    clear_modem (self);<br>
     g_clear_object (&(self->priv->object_manager)<wbr>);<br>
 }<br>
<br>
@@ -391,10 +403,10 @@ mm_device_create_modem (MMDevice                  *self,<br>
         self->priv->object_manager = g_object_ref (object_manager);<br>
<br>
         /* We want to get notified when the modem becomes valid/invalid */<br>
-        g_signal_connect (self->priv->modem,<br>
-                          "notify::" MM_BASE_MODEM_VALID,<br>
-                          G_CALLBACK (modem_valid),<br>
-                          self);<br>
+        self->priv->modem_valid_id = g_signal_connect (self->priv->modem,<br>
+                                                       "notify::" MM_BASE_MODEM_VALID,<br>
+                                                       G_CALLBACK (modem_valid),<br>
+                                                       self);<br>
     }<br>
<br>
     return !!self->priv->modem;<br>
@@ -582,7 +594,7 @@ set_property (GObject *object,<br>
         self->priv->plugin = g_value_dup_object (value);<br>
         break;<br>
     case PROP_MODEM:<br>
-        g_clear_object (&(self->priv->modem));<br>
+        clear_modem (self);<br>
         self->priv->modem = g_value_dup_object (value);<br>
         break;<br>
     case PROP_HOTPLUGGED:<br>
@@ -635,7 +647,8 @@ dispose (GObject *object)<br>
     g_clear_object (&(self->priv->plugin));<br>
     g_list_free_full (self->priv->port_probes, (GDestroyNotify)g_object_<wbr>unref);<br>
     g_list_free_full (self->priv->ignored_port_<wbr>probes, (GDestroyNotify)g_object_<wbr>unref);<br>
-    g_clear_object (&(self->priv->modem));<br>
+<br>
+    clear_modem (self);<br>
<br>
     G_OBJECT_CLASS (mm_device_parent_class)-><wbr>dispose (object);<br>
 }<br>
--<br>
2.11.1<br>
</blockquote></div><br></div></div></div>