ModemManager: MMCallbackInfo scheduled twice when device removed

[Aleksander Morgado <aleksander lanedo com>]

Hi all,

Each MMCallbackInfo holds a weak reference to the MMModem to which the
AT command was sent. When the MMModem is destroyed,
mm-callback-info.c::modem_destroyed_cb() gets called and the modem
pointer in the callback info is reset to NULL, to avoid having a pointer
to an already disposed MMModem. See [1].

But, in addition to setting the modem pointer to NULL, an explicit error
is just set directly in the callback info, and it gets scheduled. The
problem here is that after destroying the modem, the ports it held also
get closed as part of the destruction, and during port closing there's
the task of finalizing all pending commands (see
mm-serial-port.c::mm_serial_port_close()), which is done by passing an
error to the specific response handler, and which will then (usually)
propagate that error to the MMCallbackInfo and schedule it right away.

That ends up in a memory leak (most response handlers will just set
info->error without assuming there may already be a GError set); plus a
critical in the second call to mm_callback_info_schedule() because a
callback info can't be scheduled twice.

I've been looking at this problem some time, and the best way to fix it
seems to be to just avoid setting the first error and scheduling in
modem_destroyed_cb():

diff --git a/src/mm-callback-info.c b/src/mm-callback-info.c
index 1986bb5..8f8ecca 100644
--- a/src/mm-callback-info.c
+++ b/src/mm-callback-info.c
@@ -53,13 +53,9 @@ modem_destroyed_cb (gpointer data, GObject
*destroyed)
 {
     MMCallbackInfo *info = data;
 
+    /* Just set the modem pointer to NULL, do not further process the
+     * callback info */
     info->modem = NULL;
-    if (!info->pending_id) {
-        info->error = g_error_new_literal (MM_MODEM_ERROR,
-                                           MM_MODEM_ERROR_REMOVED,
-                                           "The modem was removed.");
-        mm_callback_info_schedule (info);
-    }
 }

Comments?


[1] c0253c7c293148a0bdb6c20d4b38a08401d8e34d 

-- 
Aleksander