port-qmi: implement the new generic 'removed' signal

Letting the MMBroadbandModemQmi listen for the notifications of the
QmiDevice was not a good idea, especially since no explicit reference
to the device object was hold in the modem object. This leads to race
conditions in which the QmiDevice outlives the MMBroadbandModemQmi and
the MMPortQmi, and when the "device-removed" signal is triggered, the
program crashes.

The MMPortQmi will now emit its own 'removed' signals when the
underlying QmiDevice emits "device-removed', ensuring that the signal
handler is properly cleared up during the MMPortQmi disposal.
This commit is contained in:
Aleksander Morgado
2022-12-15 21:45:53 +00:00
parent 455c486091
commit a20f2428ee
3 changed files with 27 additions and 97 deletions

View File

@@ -12873,60 +12873,6 @@ parent_initialization_started (GTask *task)
task);
}
static void
qmi_device_removed_cb (QmiDevice *device,
MMBroadbandModemQmi *self)
{
/* Reprobe the modem here so we can get notifications back. */
mm_obj_msg (self, "connection to qmi-proxy for %s lost, reprobing",
qmi_device_get_path_display (device));
g_signal_handler_disconnect (device, self->priv->qmi_device_removed_id);
self->priv->qmi_device_removed_id = 0;
mm_base_modem_set_reprobe (MM_BASE_MODEM (self), TRUE);
mm_base_modem_set_valid (MM_BASE_MODEM (self), FALSE);
}
static gboolean
track_qmi_device_removed (MMBroadbandModemQmi *self,
MMPortQmi *qmi,
GError **error)
{
QmiDevice *device;
device = mm_port_qmi_peek_device (qmi);
if (!device) {
g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
"Cannot track QMI device removal: QMI port no longer available");
return FALSE;
}
self->priv->qmi_device_removed_id = g_signal_connect (
device,
QMI_DEVICE_SIGNAL_REMOVED,
G_CALLBACK (qmi_device_removed_cb),
self);
return TRUE;
}
static void
untrack_qmi_device_removed (MMBroadbandModemQmi *self,
MMPortQmi *qmi)
{
QmiDevice *device;
if (self->priv->qmi_device_removed_id == 0)
return;
device = mm_port_qmi_peek_device (qmi);
if (!device)
return;
g_signal_handler_disconnect (device, self->priv->qmi_device_removed_id);
self->priv->qmi_device_removed_id = 0;
}
static void allocate_next_client (GTask *task);
static void
@@ -12956,20 +12902,10 @@ static void
allocate_next_client (GTask *task)
{
InitializationStartedContext *ctx;
MMBroadbandModemQmi *self;
self = g_task_get_source_object (task);
ctx = g_task_get_task_data (task);
if (ctx->service_index == G_N_ELEMENTS (qmi_services)) {
GError *error = NULL;
/* Done we are, track device removal and launch next step */
if (!track_qmi_device_removed (self, ctx->qmi, &error)) {
g_task_return_error (task, error);
g_object_unref (task);
return;
}
parent_initialization_started (task);
return;
}
@@ -13086,7 +13022,6 @@ initialization_started (MMBroadbandModem *self,
{
InitializationStartedContext *ctx;
GTask *task;
GError *error = NULL;
ctx = g_new0 (InitializationStartedContext, 1);
ctx->qmi = mm_broadband_modem_qmi_get_port_qmi (MM_BROADBAND_MODEM_QMI (self));
@@ -13105,14 +13040,6 @@ initialization_started (MMBroadbandModem *self,
}
if (mm_port_qmi_is_open (ctx->qmi)) {
/* Nothing to be done, just track device removal and launch parent's
* callback */
if (!track_qmi_device_removed (MM_BROADBAND_MODEM_QMI (self), ctx->qmi, &error)) {
g_task_return_error (task, error);
g_object_unref (task);
return;
}
parent_initialization_started (task);
return;
}
@@ -13183,8 +13110,6 @@ dispose (GObject *object)
* that will remove all port references right away */
qmi = mm_broadband_modem_qmi_peek_port_qmi (self);
if (qmi) {
/* Disconnect signal handler for qmi-proxy disappearing, if it exists */
untrack_qmi_device_removed (self, qmi);
/* If we did open the QMI port during initialization, close it now */
if (mm_port_qmi_is_open (qmi))
mm_port_qmi_close (qmi, NULL, NULL);

View File

@@ -64,8 +64,9 @@ struct _MMPortQmiPrivate {
QrtrNode *node;
#endif
/* timeout monitoring */
/* port monitoring */
gulong timeout_monitoring_id;
gulong removed_monitoring_id;
/* endpoint info */
QmiDataEndpointType endpoint_type;
gint endpoint_interface_number;
@@ -131,16 +132,6 @@ mm_port_qmi_get_client (MMPortQmi *self,
/*****************************************************************************/
QmiDevice *
mm_port_qmi_peek_device (MMPortQmi *self)
{
g_return_val_if_fail (MM_IS_PORT_QMI (self), NULL);
return self->priv->qmi_device;
}
/*****************************************************************************/
static void
initialize_endpoint_info (MMPortQmi *self)
{
@@ -196,13 +187,17 @@ mm_port_qmi_get_endpoint_info (MMPortQmi *self, MMQmiDataEndpoint *out_endpoint)
/*****************************************************************************/
static void
reset_timeout_monitoring (MMPortQmi *self,
reset_monitoring (MMPortQmi *self,
QmiDevice *qmi_device)
{
if (self->priv->timeout_monitoring_id && qmi_device) {
g_signal_handler_disconnect (qmi_device, self->priv->timeout_monitoring_id);
self->priv->timeout_monitoring_id = 0;
}
if (self->priv->removed_monitoring_id && qmi_device) {
g_signal_handler_disconnect (qmi_device, self->priv->removed_monitoring_id);
self->priv->removed_monitoring_id = 0;
}
}
static void
@@ -214,18 +209,30 @@ consecutive_timeouts_updated_cb (MMPortQmi *self,
}
static void
setup_timeout_monitoring (MMPortQmi *self,
device_removed_cb (MMPortQmi *self)
{
g_signal_emit_by_name (self, MM_PORT_SIGNAL_REMOVED);
}
static void
setup_monitoring (MMPortQmi *self,
QmiDevice *qmi_device)
{
g_assert (qmi_device);
reset_timeout_monitoring (self, qmi_device);
reset_monitoring (self, qmi_device);
g_assert (!self->priv->timeout_monitoring_id);
self->priv->timeout_monitoring_id = g_signal_connect_swapped (qmi_device,
"notify::" QMI_DEVICE_CONSECUTIVE_TIMEOUTS,
G_CALLBACK (consecutive_timeouts_updated_cb),
self);
g_assert (!self->priv->removed_monitoring_id);
self->priv->removed_monitoring_id = g_signal_connect_swapped (qmi_device,
QMI_DEVICE_SIGNAL_REMOVED,
G_CALLBACK (device_removed_cb),
self);
}
/*****************************************************************************/
@@ -2532,7 +2539,7 @@ port_open_step (GTask *task)
g_assert (ctx->device);
g_assert (!self->priv->qmi_device);
self->priv->qmi_device = g_object_ref (ctx->device);
setup_timeout_monitoring (self, ctx->device);
setup_monitoring (self, ctx->device);
self->priv->in_progress = FALSE;
g_task_return_boolean (task, TRUE);
g_object_unref (task);
@@ -2672,8 +2679,8 @@ mm_port_qmi_close (MMPortQmi *self,
ctx->qmi_device = g_steal_pointer (&self->priv->qmi_device);
g_task_set_task_data (task, ctx, (GDestroyNotify)port_qmi_close_context_free);
/* Reset timeout monitoring logic */
reset_timeout_monitoring (self, ctx->qmi_device);
/* Reset monitoring logic */
reset_monitoring (self, ctx->qmi_device);
/* Release all allocated clients */
for (l = self->priv->services; l; l = g_list_next (l)) {
@@ -2804,7 +2811,7 @@ dispose (GObject *object)
g_clear_object (&self->priv->node);
#endif
/* Clear device object */
reset_timeout_monitoring (self, self->priv->qmi_device);
reset_monitoring (self, self->priv->qmi_device);
g_clear_object (&self->priv->qmi_device);
g_clear_pointer (&self->priv->net_driver, g_free);

View File

@@ -121,8 +121,6 @@ QmiClient *mm_port_qmi_get_client (MMPortQmi *self,
QmiService service,
guint flag);
QmiDevice *mm_port_qmi_peek_device (MMPortQmi *self);
typedef struct {
QmiDataEndpointType type;
guint interface_number;