Re: [MM] [PATCH] iface-modem: Fix crash in expire_signal_quality due to removed skeleton.

[Ben Chan <benchan chromium org>]

Hi Aleksander,

Yep, that works too.  How about this?

diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c

index 49a18a5..7b44d6a 100644

--- a/src/mm-iface-modem.c

+++ b/src/mm-iface-modem.c

@@ -3676,6 +3676,14 @@ mm_iface_modem_shutdown (MMIfaceModem *self)

 {

     g_return_if_fail (MM_IS_IFACE_MODEM (self));

 

+    /* Remove SignalQualityUpdateContext object to make sure any pending

+     * invocation of expire_signal_quality is canceled before the DBus skeleton

+     * is removed. */

+    if (G_LIKELY (signal_quality_update_context_quark))

+        g_object_set_qdata (G_OBJECT (self),

+                            signal_quality_update_context_quark,

+                            NULL);

+

     /* Remove SIM object */

     g_object_set (self,

                   MM_IFACE_MODEM_SIM, NULL,

Thanks,

Ben

On Wed, Jul 11, 2012 at 9:49 PM, Aleksander Morgado <aleksander lanedo com> wrote:

Hey Ben,

> expire_signal_quality() may be called after the DBus object skeleton is
> removed in mm_iface_modem_shutdown() and before
> signal_quality_update_context_free() is called due to the destruction of
> the modem object. This patch prevents expire_signal_quality() from
> accessing an already removed skeleton.

Ugly race it is :-/ So the data set with g_object_set_qdata_full() gets
actually destroyed *after* dispose() has run, right? Just wondering if
it isn't better to ensure in each interface's _shutdown() method that we
clear any data association we may have set previously, by calling again
g_object_set_qdata() with a NULL pointer. That should call the
GDestroyNotify of each data associated previously, if any, and it will
get the timeouts removed from the main loop as well. That would also
help for the cases where, due to some bug in some other place, we let a
reference to the modem object around, effectively leaving the timeouts
running even after we think everything has been disposed.

What do you think?

Cheers!

--
Aleksander