crash in mm-device.c:modem_valid

[Aleksander Morgado]

On Sat, Feb 11, 2017 at 1:07 AM, Ben Chan <benchan at chromium.org> wrote:
> I encountered a crash in mm-device.c:modem_valid:345 where 'self->priv'
> seemed invalid, but 'modem' looked fine. As there is no code to clear the
> "notify::base-modem-valid" signal in MMDevice, I think MMDevice currently
> relies on auto disconnection of the signal when MMBaseModem is destroyed.

If we cannot rely on the Modem object outliving the Device (which I
don't think we can given that whenever an async method runs it takes a
modem object reference), the device object should
g_signal_handler_disconnect() that specific signal in dispose() when
its own modem reference is destroyed.

> However, there seems to be a circular reference between MMBaseModem and
> MMBaseSim (as also indicated in mm_device_remove_modem), so it seems like
> MMDevice::dispose doesn't break the cycle.
>

It's not about breaking any cycle I think; as I said, just think of
any mm_base_modem_at_command() async operation where there is a
reference to the modem object obtained; in that case disposing the
device will end up leaving the signal still connected to the still
valid modem object, while the device is no longer valid.

As for the reference within the SIM object, that is another thing,
although may have been related as well. The logic says that the modem
object owns the SIM object, and the device object owns the modem
object. The difference between these is that the SIM object requires
the modem object (e.g. for AT/QMI/MBIM commands); while the modem
object doesn't require anything from the parent device object. With
the weak reference we would be able to reset the internal modem
pointer stored within the SIM object to NULL when the modem object is
totally unref-ed, but then we would need to change all the logic
everywhere in the MMBaseSim object and other subclasses to support the
case of having a NULL modem pointer... So that's why we have a full
reference to the parent modem object in the SIM object, so that we can
unconditionally use the internal modem object as long as the SIM
object is around.

Let me prepare a patch and suggest it here for you to review.

-- 
Aleksander
https://aleksander.es