From c7dac253158394389e0428180b27b0eadb64cd37 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 15 Dec 2022 22:44:25 +0000 Subject: [PATCH] port-mbim: chain up device notifications through the port Instead of having the modem object listen notifications in the MbimDevice, the MMPortMbim will act as an intermediate emitter for the same. The primary benefit is that the lifecycle of the port object is tightly coupled to the life of the modem object, so there is no chance that the port object outlives the modem object. And the port object will ensure its own device notification handler is correctly cleared up on its dispose, so there should be no chance of firing up a signal in the device for a listener that is already freed. --- src/mm-broadband-modem-mbim.c | 72 ++++++++++++++++++++++------------- src/mm-port-mbim.c | 34 +++++++++++++++++ src/mm-port-mbim.h | 6 +++ 3 files changed, 85 insertions(+), 27 deletions(-) diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-modem-mbim.c index 9a13a88b..067728c1 100644 --- a/src/mm-broadband-modem-mbim.c +++ b/src/mm-broadband-modem-mbim.c @@ -5340,11 +5340,17 @@ ussd_notification (MMBroadbandModemMbim *self, } static void -device_notification_cb (MbimDevice *device, - MbimMessage *notification, - MMBroadbandModemMbim *self) +port_notification_cb (MMPortMbim *port, + MbimMessage *notification, + MMBroadbandModemMbim *self) { - MbimService service; + MbimService service; + MbimDevice *device; + + /* Onlyu process notifications if the device still exists */ + device = mm_port_mbim_peek_device (port); + if (!device) + return; service = mbim_message_indicate_status_get_service (notification); mm_obj_dbg (self, "received notification (service '%s', command '%s')", @@ -5364,10 +5370,10 @@ device_notification_cb (MbimDevice *device, static void common_setup_cleanup_unsolicited_events_sync (MMBroadbandModemMbim *self, - MbimDevice *device, + MMPortMbim *port, gboolean setup) { - if (!device) + if (!port) return; mm_obj_dbg (self, "supported notifications: signal (%s), registration (%s), sms (%s), " @@ -5389,16 +5395,16 @@ common_setup_cleanup_unsolicited_events_sync (MMBroadbandModemMbim *self, /* Don't re-enable it if already there */ if (!self->priv->notification_id) self->priv->notification_id = - g_signal_connect (device, - MBIM_DEVICE_SIGNAL_INDICATE_STATUS, - G_CALLBACK (device_notification_cb), + g_signal_connect (port, + MM_PORT_MBIM_SIGNAL_NOTIFICATION, + G_CALLBACK (port_notification_cb), self); } else { /* Don't remove the signal if there are still listeners interested */ if (self->priv->setup_flags == PROCESS_NOTIFICATION_FLAG_NONE && self->priv->notification_id && - g_signal_handler_is_connected (device, self->priv->notification_id)) { - g_signal_handler_disconnect (device, self->priv->notification_id); + g_signal_handler_is_connected (port, self->priv->notification_id)) { + g_signal_handler_disconnect (port, self->priv->notification_id); self->priv->notification_id = 0; } } @@ -5419,14 +5425,20 @@ common_setup_cleanup_unsolicited_events (MMBroadbandModemMbim *self, 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); + MMPortMbim *port; task = g_task_new (self, NULL, callback, user_data); + + port = mm_broadband_modem_mbim_peek_port_mbim (MM_BROADBAND_MODEM_MBIM (self)); + if (!port) { + g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, + "Couldn't peek MBIM port"); + g_object_unref (task); + return; + } + + common_setup_cleanup_unsolicited_events_sync (self, port, setup); + g_task_return_boolean (task, TRUE); g_object_unref (task); } @@ -5702,7 +5714,7 @@ modem_3gpp_enable_unsolicited_registration_events (MMIfaceModem3gpp *_self, /* Setup SIM hot swap */ typedef struct { - MbimDevice *device; + MMPortMbim *port; GError *subscriber_info_error; #if defined WITH_QMI && QMI_MBIM_QMUX_SUPPORTED GError *qmi_error; @@ -5716,7 +5728,7 @@ setup_sim_hot_swap_context_free (SetupSimHotSwapContext *ctx) g_clear_error (&ctx->qmi_error); #endif g_clear_error (&ctx->subscriber_info_error); - g_clear_object (&ctx->device); + g_clear_object (&ctx->port); g_slice_free (SetupSimHotSwapContext, ctx); } @@ -5780,7 +5792,7 @@ enable_subscriber_info_unsolicited_events_ready (MMBroadbandModemMbim *self, mm_obj_dbg (self, "failed to enable subscriber info events: %s", ctx->subscriber_info_error->message); /* reset setup flags if enabling failed */ self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; - common_setup_cleanup_unsolicited_events_sync (self, ctx->device, FALSE); + common_setup_cleanup_unsolicited_events_sync (self, ctx->port, FALSE); /* and also reset enable flags */ self->priv->enable_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; } @@ -5800,21 +5812,27 @@ modem_setup_sim_hot_swap (MMIfaceModem *_self, gpointer user_data) { MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self); - MbimDevice *device; + MMPortMbim *port; GTask *task; SetupSimHotSwapContext *ctx; - if (!peek_device (self, &device, callback, user_data)) - return; - task = g_task_new (self, NULL, callback, user_data); + + port = mm_broadband_modem_mbim_peek_port_mbim (MM_BROADBAND_MODEM_MBIM (self)); + if (!port) { + g_task_return_new_error (task, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, + "Couldn't peek MBIM port"); + g_object_unref (task); + return; + } + ctx = g_slice_new0 (SetupSimHotSwapContext); - ctx->device = g_object_ref (device); + ctx->port = g_object_ref (port); g_task_set_task_data (task, ctx, (GDestroyNotify)setup_sim_hot_swap_context_free); /* Setup flags synchronously, which never fails */ self->priv->setup_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; - common_setup_cleanup_unsolicited_events_sync (self, ctx->device, TRUE); + common_setup_cleanup_unsolicited_events_sync (self, ctx->port, TRUE); /* Enable flags asynchronously, which may fail */ self->priv->enable_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; @@ -9174,7 +9192,7 @@ dispose (GObject *object) 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); + common_setup_cleanup_unsolicited_events_sync (self, mbim, FALSE); /* If we did open the MBIM port during initialization, close it now */ if (mm_port_mbim_is_open (mbim)) diff --git a/src/mm-port-mbim.c b/src/mm-port-mbim.c index ae1021a0..e900ebce 100644 --- a/src/mm-port-mbim.c +++ b/src/mm-port-mbim.c @@ -31,11 +31,19 @@ G_DEFINE_TYPE (MMPortMbim, mm_port_mbim, MM_TYPE_PORT) +enum { + SIGNAL_NOTIFICATION, + SIGNAL_LAST +}; + +static guint signals[SIGNAL_LAST] = { 0 }; + struct _MMPortMbimPrivate { gboolean in_progress; MbimDevice *mbim_device; /* monitoring */ + gulong notification_monitoring_id; gulong timeout_monitoring_id; gulong removed_monitoring_id; @@ -396,6 +404,10 @@ static void reset_monitoring (MMPortMbim *self, MbimDevice *mbim_device) { + if (self->priv->notification_monitoring_id && mbim_device) { + g_signal_handler_disconnect (mbim_device, self->priv->notification_monitoring_id); + self->priv->notification_monitoring_id = 0; + } if (self->priv->timeout_monitoring_id && mbim_device) { g_signal_handler_disconnect (mbim_device, self->priv->timeout_monitoring_id); self->priv->timeout_monitoring_id = 0; @@ -420,6 +432,13 @@ device_removed_cb (MMPortMbim *self) g_signal_emit_by_name (self, MM_PORT_SIGNAL_REMOVED); } +static void +notification_cb (MMPortMbim *self, + MbimMessage *notification) +{ + g_signal_emit (self, signals[SIGNAL_NOTIFICATION], 0, notification); +} + static void setup_monitoring (MMPortMbim *self, MbimDevice *mbim_device) @@ -428,6 +447,12 @@ setup_monitoring (MMPortMbim *self, reset_monitoring (self, mbim_device); + g_assert (!self->priv->notification_monitoring_id); + self->priv->notification_monitoring_id = g_signal_connect_swapped (mbim_device, + MBIM_DEVICE_SIGNAL_INDICATE_STATUS, + G_CALLBACK (notification_cb), + self); + g_assert (!self->priv->timeout_monitoring_id); self->priv->timeout_monitoring_id = g_signal_connect_swapped (mbim_device, "notify::" MBIM_DEVICE_CONSECUTIVE_TIMEOUTS, @@ -928,4 +953,13 @@ mm_port_mbim_class_init (MMPortMbimClass *klass) /* Virtual methods */ object_class->dispose = dispose; + + signals[SIGNAL_NOTIFICATION] = + g_signal_new (MM_PORT_MBIM_SIGNAL_NOTIFICATION, + G_OBJECT_CLASS_TYPE (G_OBJECT_CLASS (klass)), + G_SIGNAL_RUN_LAST, + G_STRUCT_OFFSET (MMPortMbimClass, notification), + NULL, NULL, + g_cclosure_marshal_generic, + G_TYPE_NONE, 1, MBIM_TYPE_MESSAGE); } diff --git a/src/mm-port-mbim.h b/src/mm-port-mbim.h index 81e3deb7..2bc1855a 100644 --- a/src/mm-port-mbim.h +++ b/src/mm-port-mbim.h @@ -37,6 +37,8 @@ #define MM_IS_PORT_MBIM_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), MM_TYPE_PORT_MBIM)) #define MM_PORT_MBIM_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), MM_TYPE_PORT_MBIM, MMPortMbimClass)) +#define MM_PORT_MBIM_SIGNAL_NOTIFICATION "notification" + typedef struct _MMPortMbim MMPortMbim; typedef struct _MMPortMbimClass MMPortMbimClass; typedef struct _MMPortMbimPrivate MMPortMbimPrivate; @@ -48,6 +50,10 @@ struct _MMPortMbim { struct _MMPortMbimClass { MMPortClass parent; + + /* signals */ + void (* notification) (MMPortMbim *port, + MbimMessage *notification); }; GType mm_port_mbim_get_type (void);