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.
This commit is contained in:
Aleksander Morgado
2022-12-15 22:44:25 +00:00
parent 309a8a515b
commit c7dac25315
3 changed files with 85 additions and 27 deletions

View File

@@ -5340,11 +5340,17 @@ ussd_notification (MMBroadbandModemMbim *self,
} }
static void static void
device_notification_cb (MbimDevice *device, port_notification_cb (MMPortMbim *port,
MbimMessage *notification, MbimMessage *notification,
MMBroadbandModemMbim *self) 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); service = mbim_message_indicate_status_get_service (notification);
mm_obj_dbg (self, "received notification (service '%s', command '%s')", mm_obj_dbg (self, "received notification (service '%s', command '%s')",
@@ -5364,10 +5370,10 @@ device_notification_cb (MbimDevice *device,
static void static void
common_setup_cleanup_unsolicited_events_sync (MMBroadbandModemMbim *self, common_setup_cleanup_unsolicited_events_sync (MMBroadbandModemMbim *self,
MbimDevice *device, MMPortMbim *port,
gboolean setup) gboolean setup)
{ {
if (!device) if (!port)
return; return;
mm_obj_dbg (self, "supported notifications: signal (%s), registration (%s), sms (%s), " 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 */ /* Don't re-enable it if already there */
if (!self->priv->notification_id) if (!self->priv->notification_id)
self->priv->notification_id = self->priv->notification_id =
g_signal_connect (device, g_signal_connect (port,
MBIM_DEVICE_SIGNAL_INDICATE_STATUS, MM_PORT_MBIM_SIGNAL_NOTIFICATION,
G_CALLBACK (device_notification_cb), G_CALLBACK (port_notification_cb),
self); self);
} else { } else {
/* Don't remove the signal if there are still listeners interested */ /* Don't remove the signal if there are still listeners interested */
if (self->priv->setup_flags == PROCESS_NOTIFICATION_FLAG_NONE && if (self->priv->setup_flags == PROCESS_NOTIFICATION_FLAG_NONE &&
self->priv->notification_id && self->priv->notification_id &&
g_signal_handler_is_connected (device, self->priv->notification_id)) { g_signal_handler_is_connected (port, self->priv->notification_id)) {
g_signal_handler_disconnect (device, self->priv->notification_id); g_signal_handler_disconnect (port, self->priv->notification_id);
self->priv->notification_id = 0; self->priv->notification_id = 0;
} }
} }
@@ -5419,14 +5425,20 @@ common_setup_cleanup_unsolicited_events (MMBroadbandModemMbim *self,
gpointer user_data) gpointer user_data)
{ {
GTask *task; GTask *task;
MbimDevice *device; MMPortMbim *port;
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); 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_task_return_boolean (task, TRUE);
g_object_unref (task); g_object_unref (task);
} }
@@ -5702,7 +5714,7 @@ modem_3gpp_enable_unsolicited_registration_events (MMIfaceModem3gpp *_self,
/* Setup SIM hot swap */ /* Setup SIM hot swap */
typedef struct { typedef struct {
MbimDevice *device; MMPortMbim *port;
GError *subscriber_info_error; GError *subscriber_info_error;
#if defined WITH_QMI && QMI_MBIM_QMUX_SUPPORTED #if defined WITH_QMI && QMI_MBIM_QMUX_SUPPORTED
GError *qmi_error; GError *qmi_error;
@@ -5716,7 +5728,7 @@ setup_sim_hot_swap_context_free (SetupSimHotSwapContext *ctx)
g_clear_error (&ctx->qmi_error); g_clear_error (&ctx->qmi_error);
#endif #endif
g_clear_error (&ctx->subscriber_info_error); g_clear_error (&ctx->subscriber_info_error);
g_clear_object (&ctx->device); g_clear_object (&ctx->port);
g_slice_free (SetupSimHotSwapContext, ctx); 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); mm_obj_dbg (self, "failed to enable subscriber info events: %s", ctx->subscriber_info_error->message);
/* reset setup flags if enabling failed */ /* reset setup flags if enabling failed */
self->priv->setup_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; 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 */ /* and also reset enable flags */
self->priv->enable_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; self->priv->enable_flags &= ~PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
} }
@@ -5800,21 +5812,27 @@ modem_setup_sim_hot_swap (MMIfaceModem *_self,
gpointer user_data) gpointer user_data)
{ {
MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self); MMBroadbandModemMbim *self = MM_BROADBAND_MODEM_MBIM (_self);
MbimDevice *device; MMPortMbim *port;
GTask *task; GTask *task;
SetupSimHotSwapContext *ctx; SetupSimHotSwapContext *ctx;
if (!peek_device (self, &device, callback, user_data))
return;
task = g_task_new (self, NULL, callback, user_data); 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 = 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); g_task_set_task_data (task, ctx, (GDestroyNotify)setup_sim_hot_swap_context_free);
/* Setup flags synchronously, which never fails */ /* Setup flags synchronously, which never fails */
self->priv->setup_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; 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 */ /* Enable flags asynchronously, which may fail */
self->priv->enable_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO; self->priv->enable_flags |= PROCESS_NOTIFICATION_FLAG_SUBSCRIBER_INFO;
@@ -9174,7 +9192,7 @@ dispose (GObject *object)
if (mbim) { if (mbim) {
/* Explicitly remove notification handler */ /* Explicitly remove notification handler */
self->priv->setup_flags = PROCESS_NOTIFICATION_FLAG_NONE; 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 we did open the MBIM port during initialization, close it now */
if (mm_port_mbim_is_open (mbim)) if (mm_port_mbim_is_open (mbim))

View File

@@ -31,11 +31,19 @@
G_DEFINE_TYPE (MMPortMbim, mm_port_mbim, MM_TYPE_PORT) G_DEFINE_TYPE (MMPortMbim, mm_port_mbim, MM_TYPE_PORT)
enum {
SIGNAL_NOTIFICATION,
SIGNAL_LAST
};
static guint signals[SIGNAL_LAST] = { 0 };
struct _MMPortMbimPrivate { struct _MMPortMbimPrivate {
gboolean in_progress; gboolean in_progress;
MbimDevice *mbim_device; MbimDevice *mbim_device;
/* monitoring */ /* monitoring */
gulong notification_monitoring_id;
gulong timeout_monitoring_id; gulong timeout_monitoring_id;
gulong removed_monitoring_id; gulong removed_monitoring_id;
@@ -396,6 +404,10 @@ static void
reset_monitoring (MMPortMbim *self, reset_monitoring (MMPortMbim *self,
MbimDevice *mbim_device) 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) { if (self->priv->timeout_monitoring_id && mbim_device) {
g_signal_handler_disconnect (mbim_device, self->priv->timeout_monitoring_id); g_signal_handler_disconnect (mbim_device, self->priv->timeout_monitoring_id);
self->priv->timeout_monitoring_id = 0; 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); 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 static void
setup_monitoring (MMPortMbim *self, setup_monitoring (MMPortMbim *self,
MbimDevice *mbim_device) MbimDevice *mbim_device)
@@ -428,6 +447,12 @@ setup_monitoring (MMPortMbim *self,
reset_monitoring (self, mbim_device); 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); g_assert (!self->priv->timeout_monitoring_id);
self->priv->timeout_monitoring_id = g_signal_connect_swapped (mbim_device, self->priv->timeout_monitoring_id = g_signal_connect_swapped (mbim_device,
"notify::" MBIM_DEVICE_CONSECUTIVE_TIMEOUTS, "notify::" MBIM_DEVICE_CONSECUTIVE_TIMEOUTS,
@@ -928,4 +953,13 @@ mm_port_mbim_class_init (MMPortMbimClass *klass)
/* Virtual methods */ /* Virtual methods */
object_class->dispose = dispose; 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);
} }

View File

@@ -37,6 +37,8 @@
#define MM_IS_PORT_MBIM_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), MM_TYPE_PORT_MBIM)) #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_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 _MMPortMbim MMPortMbim;
typedef struct _MMPortMbimClass MMPortMbimClass; typedef struct _MMPortMbimClass MMPortMbimClass;
typedef struct _MMPortMbimPrivate MMPortMbimPrivate; typedef struct _MMPortMbimPrivate MMPortMbimPrivate;
@@ -48,6 +50,10 @@ struct _MMPortMbim {
struct _MMPortMbimClass { struct _MMPortMbimClass {
MMPortClass parent; MMPortClass parent;
/* signals */
void (* notification) (MMPortMbim *port,
MbimMessage *notification);
}; };
GType mm_port_mbim_get_type (void); GType mm_port_mbim_get_type (void);