[PATCH] broadband-modem-mbim: explicitly remove notification handler on unref

[Dan Williams]

On Fri, 2017-08-11 at 17:04 +0200, Aleksander Morgado wrote:
> When we remove the last object reference, make sure the notification
> handler is also removed, or we may end up using an already freed
> object.
> 
> https://retrace.fedoraproject.org/faf/reports/1815001/

Good catch; any time we connect a signal somewhere in an object, we
should probably have an explicit disconnect/clear of that handler in
the finalize/dispose path just to be sure.

Which brings up the question, should that kind of thing go in dispose? 
Honestly I don't think it makes much of a difference, except that if
it's in finalize we might get the handler triggered during dispose when
half the object is cleaned up already.

Dan

> ---
>  src/mm-broadband-modem-mbim.c | 61 +++++++++++++++++++++++++++----
> ------------
>  1 file changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-
> mbim.c
> index 815f5455..8a722b11 100644
> --- a/src/mm-broadband-modem-mbim.c
> +++ b/src/mm-broadband-modem-mbim.c
> @@ -2215,24 +2215,12 @@ device_notification_cb (MbimDevice *device,
>      }
>  }
>  
> -static gboolean
> -common_setup_cleanup_unsolicited_events_finish (MMBroadbandModemMbim
> *self,
> -                                                GAsyncResult *res,
> -                                                GError **error)
> -{
> -    return g_task_propagate_boolean (G_TASK (res), error);
> -}
> -
>  static void
> -common_setup_cleanup_unsolicited_events (MMBroadbandModemMbim *self,
> -                                         gboolean setup,
> -                                         GAsyncReadyCallback
> callback,
> -                                         gpointer user_data)
> +common_setup_cleanup_unsolicited_events_sync (MMBroadbandModemMbim
> *self,
> +                                              MbimDevice           *
> device,
> +                                              gboolean              
> setup)
>  {
> -    MbimDevice *device;
> -    GTask *task;
> -
> -    if (!peek_device (self, &device, callback, user_data))
> +    if (!device)
>          return;
>  
>      mm_dbg ("Supported notifications: signal (%s), registration
> (%s), sms (%s), connect (%s), subscriber (%s), packet (%s)",
> @@ -2260,6 +2248,29 @@ common_setup_cleanup_unsolicited_events
> (MMBroadbandModemMbim *self,
>              self->priv->notification_id = 0;
>          }
>      }
> +}
> +
> +static gboolean
> +common_setup_cleanup_unsolicited_events_finish
> (MMBroadbandModemMbim  *self,
> +                                                GAsyncResult        
>   *res,
> +                                                GError              
>  **error)
> +{
> +    return g_task_propagate_boolean (G_TASK (res), error);
> +}
> +
> +static void
> +common_setup_cleanup_unsolicited_events (MMBroadbandModemMbim *self,
> +                                         gboolean              setup
> ,
> +                                         GAsyncReadyCallback   callb
> ack,
> +                                         gpointer              user_
> data)
> +{
> +    GTask      *task;
> +    MbimDevice *device;
> +
> +    if (!peek_device (self, &device, callback, user_data))
> +        return;
> +
> +    common_setup_cleanup_unsolicited_events_sync (self, device,
> setup);
>  
>      task = g_task_new (self, NULL, callback, user_data);
>      g_task_return_boolean (task, TRUE);
> @@ -3174,20 +3185,24 @@ mm_broadband_modem_mbim_init
> (MMBroadbandModemMbim *self)
>  static void
>  finalize (GObject *object)
>  {
> -    MMPortMbim *mbim;
>      MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (object);
> +    MMPortMbim *mbim;
> +
> +    mbim = mm_base_modem_peek_port_mbim (MM_BASE_MODEM (self));
> +    if (mbim) {
> +        /* Explicitly remove notification handler */
> +        self->priv->setup_flags = PROCESS_NOTIFICATION_FLAG_NONE;
> +        common_setup_cleanup_unsolicited_events_sync (self,
> mm_port_mbim_peek_device (mbim), FALSE);
> +        /* If we did open the MBIM port during initialization, close
> it now */
> +        if (mm_port_mbim_is_open (mbim))
> +            mm_port_mbim_close (mbim, NULL, NULL);
> +    }
>  
>      g_free (self->priv->caps_device_id);
>      g_free (self->priv->caps_firmware_info);
>      g_free (self->priv->current_operator_id);
>      g_free (self->priv->current_operator_name);
>  
> -    mbim = mm_base_modem_peek_port_mbim (MM_BASE_MODEM (self));
> -    /* If we did open the MBIM port during initialization, close it
> now */
> -    if (mbim && mm_port_mbim_is_open (mbim)) {
> -        mm_port_mbim_close (mbim, NULL, NULL);
> -    }
> -
>      G_OBJECT_CLASS (mm_broadband_modem_mbim_parent_class)->finalize
> (object);
>  }
>