Hi Aleksander, 2016-04-08 16:42 GMT+02:00 Aleksander Morgado <aleksan...@aleksander.es>: > When we were completing tasks in idle, the logic was like this: > > * Schedule task completion in idle > * self->priv->task = NULL > * (idle) Task completion callback called > > This meant that the self->priv->task was always set to NULL before the > completion callback was called, which is what we wanted as a new task may be > scheduled in the callback itself. > > Now, without completing in idle, we were wrongly doing: > > * Task completion callback called > * self->priv->task = NULL > > This commit fixes the logic by making sure self->priv->task = NULL before any > task completion. > --- > > Daniele, can you validate that this patch fixes your issue? >
According to a quick test I did, it seems to fix the issue on LE910. On Monday I will do more testing. Thank you very much for your help! Regards, Daniele > --- > src/mm-port-probe.c | 184 > ++++++++++++++++++++++++++++------------------------ > 1 file changed, 100 insertions(+), 84 deletions(-) > > diff --git a/src/mm-port-probe.c b/src/mm-port-probe.c > index a1ca8ba..5d2428a 100644 > --- a/src/mm-port-probe.c > +++ b/src/mm-port-probe.c > @@ -95,6 +95,50 @@ struct _MMPortProbePrivate { > }; > > > /*****************************************************************************/ > +/* Probe task completions. > + * Always make sure that the stored task is NULL when the task is completed. > + */ > + > +static gboolean > +port_probe_task_return_error_if_cancelled (MMPortProbe *self) > +{ > + GTask *task; > + > + task = self->priv->task; > + self->priv->task = NULL; > + > + if (g_task_return_error_if_cancelled (task)) { > + g_object_unref (task); > + return TRUE; > + } > + > + self->priv->task = task; > + return FALSE; > +} > + > +static void > +port_probe_task_return_error (MMPortProbe *self, > + GError *error) > +{ > + GTask *task; > + > + task = self->priv->task; > + self->priv->task = NULL; > + g_task_return_error (task, error); > +} > + > +static void > +port_probe_task_return_boolean (MMPortProbe *self, > + gboolean result) > +{ > + GTask *task; > + > + task = self->priv->task; > + self->priv->task = NULL; > + g_task_return_boolean (task, result); > +} > + > +/*****************************************************************************/ > > void > mm_port_probe_set_result_at (MMPortProbe *self, > @@ -527,10 +571,8 @@ wdm_probe (MMPortProbe *self) > ctx->source_id = 0; > > /* If already cancelled, do nothing else */ > - if (g_task_return_error_if_cancelled (self->priv->task)) { > - g_clear_object (&self->priv->task); > + if (port_probe_task_return_error_if_cancelled (self)) > return G_SOURCE_REMOVE; > - } > > /* QMI probing needed? */ > if ((ctx->flags & MM_PORT_PROBE_QMI) && > @@ -547,8 +589,7 @@ wdm_probe (MMPortProbe *self) > } > > /* All done now */ > - g_task_return_boolean (self->priv->task, TRUE); > - g_clear_object (&self->priv->task); > + port_probe_task_return_boolean (self, TRUE); > return G_SOURCE_REMOVE; > } > > @@ -571,10 +612,8 @@ serial_probe_qcdm_parse_response (MMPortSerialQcdm *port, > ctx = g_task_get_task_data (self->priv->task); > > /* If already cancelled, do nothing else */ > - if (g_task_return_error_if_cancelled (self->priv->task)) { > - g_clear_object (&self->priv->task); > + if (port_probe_task_return_error_if_cancelled (self)) > return; > - } > > response = mm_port_serial_qcdm_command_finish (port, res, &error); > if (!error) { > @@ -642,10 +681,8 @@ serial_probe_qcdm (MMPortProbe *self) > ctx->source_id = 0; > > /* If already cancelled, do nothing else */ > - if (g_task_return_error_if_cancelled (self->priv->task)) { > - g_clear_object (&self->priv->task); > + if (port_probe_task_return_error_if_cancelled (self)) > return G_SOURCE_REMOVE; > - } > > mm_dbg ("(%s/%s) probing QCDM...", > g_udev_device_get_subsystem (self->priv->port), > @@ -665,27 +702,25 @@ serial_probe_qcdm (MMPortProbe *self) > /* Open the QCDM port */ > ctx->serial = MM_PORT_SERIAL (mm_port_serial_qcdm_new > (g_udev_device_get_name (self->priv->port))); > if (!ctx->serial) { > - g_task_return_new_error (self->priv->task, > - MM_CORE_ERROR, > - MM_CORE_ERROR_FAILED, > - "(%s/%s) Couldn't create QCDM port", > - g_udev_device_get_subsystem > (self->priv->port), > - g_udev_device_get_name (self->priv->port)); > - g_clear_object (&self->priv->task); > + port_probe_task_return_error (self, > + g_error_new (MM_CORE_ERROR, > + MM_CORE_ERROR_FAILED, > + "(%s/%s) Couldn't create > QCDM port", > + > g_udev_device_get_subsystem (self->priv->port), > + g_udev_device_get_name > (self->priv->port))); > return G_SOURCE_REMOVE; > } > > /* Try to open the port */ > if (!mm_port_serial_open (ctx->serial, &error)) { > - g_task_return_new_error (self->priv->task, > - MM_SERIAL_ERROR, > - MM_SERIAL_ERROR_OPEN_FAILED, > - "(%s/%s) Failed to open QCDM port: %s", > - g_udev_device_get_subsystem > (self->priv->port), > - g_udev_device_get_name (self->priv->port), > - (error ? error->message : "unknown error")); > + port_probe_task_return_error (self, > + g_error_new (MM_SERIAL_ERROR, > + > MM_SERIAL_ERROR_OPEN_FAILED, > + "(%s/%s) Failed to open > QCDM port: %s", > + > g_udev_device_get_subsystem (self->priv->port), > + g_udev_device_get_name > (self->priv->port), > + (error ? error->message : > "unknown error"))); > g_clear_error (&error); > - g_clear_object (&self->priv->task); > return G_SOURCE_REMOVE; > } > > @@ -699,13 +734,12 @@ serial_probe_qcdm (MMPortProbe *self) > len = qcdm_cmd_version_info_new ((char *) (verinfo->data + 1), 9); > if (len <= 0) { > g_byte_array_unref (verinfo); > - g_task_return_new_error (self->priv->task, > - MM_SERIAL_ERROR, > - MM_SERIAL_ERROR_OPEN_FAILED, > - "(%s/%s) Failed to create QCDM versin info > command", > - g_udev_device_get_subsystem > (self->priv->port), > - g_udev_device_get_name (self->priv->port)); > - g_clear_object (&self->priv->task); > + port_probe_task_return_error (self, > + g_error_new (MM_SERIAL_ERROR, > + > MM_SERIAL_ERROR_OPEN_FAILED, > + "(%s/%s) Failed to create > QCDM versin info command", > + > g_udev_device_get_subsystem (self->priv->port), > + g_udev_device_get_name > (self->priv->port))); > return G_SOURCE_REMOVE; > } > verinfo->len = len + 1; > @@ -857,10 +891,8 @@ serial_probe_at_parse_response (MMPortSerialAt *port, > ctx = g_task_get_task_data (self->priv->task); > > /* If already cancelled, do nothing else */ > - if (g_task_return_error_if_cancelled (self->priv->task)) { > - g_clear_object (&self->priv->task); > + if (port_probe_task_return_error_if_cancelled (self)) > return; > - } > > /* If AT probing cancelled, end this partial probing */ > if (g_cancellable_is_cancelled (ctx->at_probing_cancellable)) { > @@ -882,14 +914,13 @@ serial_probe_at_parse_response (MMPortSerialAt *port, > &result_error)) { > /* Were we told to abort the whole probing? */ > if (result_error) { > - g_task_return_new_error (self->priv->task, > - MM_CORE_ERROR, > - MM_CORE_ERROR_UNSUPPORTED, > - "(%s/%s) error while probing AT > features: %s", > - g_udev_device_get_subsystem > (self->priv->port), > - g_udev_device_get_name > (self->priv->port), > - result_error->message); > - g_clear_object (&self->priv->task); > + port_probe_task_return_error (self, > + g_error_new (MM_CORE_ERROR, > + > MM_CORE_ERROR_UNSUPPORTED, > + "(%s/%s) error while > probing AT features: %s", > + > g_udev_device_get_subsystem (self->priv->port), > + > g_udev_device_get_name (self->priv->port), > + > result_error->message)); > goto out; > } > > @@ -940,10 +971,8 @@ serial_probe_at (MMPortProbe *self) > ctx->source_id = 0; > > /* If already cancelled, do nothing else */ > - if (g_task_return_error_if_cancelled (self->priv->task)) { > - g_clear_object (&self->priv->task); > + if (port_probe_task_return_error_if_cancelled (self)) > return G_SOURCE_REMOVE; > - } > > /* If AT probing cancelled, end this partial probing */ > if (g_cancellable_is_cancelled (ctx->at_probing_cancellable)) { > @@ -1007,8 +1036,7 @@ at_custom_init_ready (MMPortProbe *self, > > if (!ctx->at_custom_init_finish (self, res, &error)) { > /* All errors propagated up end up forcing an UNSUPPORTED result */ > - g_task_return_error (self->priv->task, error); > - g_clear_object (&self->priv->task); > + port_probe_task_return_error (self, error); > return; > } > > @@ -1023,16 +1051,13 @@ static void > serial_probe_schedule (MMPortProbe *self) > { > PortProbeRunContext *ctx; > - GTask *task; > > g_assert (self->priv->task); > ctx = g_task_get_task_data (self->priv->task); > > /* If already cancelled, do nothing else */ > - if (g_task_return_error_if_cancelled (self->priv->task)) { > - g_clear_object (&self->priv->task); > + if (port_probe_task_return_error_if_cancelled (self)) > return; > - } > > /* If we got some custom initialization setup requested, go on with it > * first. */ > @@ -1101,10 +1126,7 @@ serial_probe_schedule (MMPortProbe *self) > } > > /* All done! */ > - task = self->priv->task; > - self->priv->task = NULL; > - g_task_return_boolean (task, TRUE); > - g_object_unref (task); > + port_probe_task_return_boolean (self, TRUE); > } > > static void > @@ -1168,10 +1190,8 @@ serial_open_at (MMPortProbe *self) > ctx->source_id = 0; > > /* If already cancelled, do nothing else */ > - if (g_task_return_error_if_cancelled (self->priv->task)) { > - g_clear_object (&self->priv->task); > + if (port_probe_task_return_error_if_cancelled (self)) > return G_SOURCE_REMOVE; > - } > > /* Create AT serial port if not done before */ > if (!ctx->serial) { > @@ -1183,13 +1203,12 @@ serial_open_at (MMPortProbe *self) > > ctx->serial = MM_PORT_SERIAL (mm_port_serial_at_new > (g_udev_device_get_name (self->priv->port), subsys)); > if (!ctx->serial) { > - g_task_return_new_error (self->priv->task, > - MM_CORE_ERROR, > - MM_CORE_ERROR_FAILED, > - "(%s/%s) couldn't create AT port", > - g_udev_device_get_subsystem > (self->priv->port), > - g_udev_device_get_name > (self->priv->port)); > - g_clear_object (&self->priv->task); > + port_probe_task_return_error (self, > + g_error_new (MM_CORE_ERROR, > + MM_CORE_ERROR_FAILED, > + "(%s/%s) couldn't > create AT port", > + > g_udev_device_get_subsystem (self->priv->port), > + > g_udev_device_get_name (self->priv->port))); > return G_SOURCE_REMOVE; > } > > @@ -1215,13 +1234,12 @@ serial_open_at (MMPortProbe *self) > /* Abort if maximum number of open tries reached */ > if (++ctx->at_open_tries > 4) { > /* took too long to open the port; give up */ > - g_task_return_new_error (self->priv->task, > - MM_CORE_ERROR, > - MM_CORE_ERROR_FAILED, > - "(%s/%s) failed to open port after 4 > tries", > - g_udev_device_get_subsystem > (self->priv->port), > - g_udev_device_get_name > (self->priv->port)); > - g_clear_object (&self->priv->task); > + port_probe_task_return_error (self, > + g_error_new (MM_CORE_ERROR, > + MM_CORE_ERROR_FAILED, > + "(%s/%s) failed to > open port after 4 tries", > + > g_udev_device_get_subsystem (self->priv->port), > + > g_udev_device_get_name (self->priv->port))); > g_clear_error (&error); > return G_SOURCE_REMOVE; > } > @@ -1233,14 +1251,13 @@ serial_open_at (MMPortProbe *self) > return G_SOURCE_REMOVE; > } > > - g_task_return_new_error (self->priv->task, > - MM_SERIAL_ERROR, > - MM_SERIAL_ERROR_OPEN_FAILED, > - "(%s/%s) failed to open port: %s", > - g_udev_device_get_subsystem > (self->priv->port), > - g_udev_device_get_name (self->priv->port), > - (error ? error->message : "unknown error")); > - g_clear_object (&self->priv->task); > + port_probe_task_return_error (self, > + g_error_new (MM_SERIAL_ERROR, > + > MM_SERIAL_ERROR_OPEN_FAILED, > + "(%s/%s) failed to open > port: %s", > + > g_udev_device_get_subsystem (self->priv->port), > + g_udev_device_get_name > (self->priv->port), > + (error ? error->message : > "unknown error"))); > g_clear_error (&error); > return G_SOURCE_REMOVE; > } > @@ -1346,8 +1363,7 @@ mm_port_probe_run (MMPortProbe *self, > mm_dbg ("(%s/%s) port probing finished: no more probings needed", > g_udev_device_get_subsystem (self->priv->port), > g_udev_device_get_name (self->priv->port)); > - g_task_return_boolean (self->priv->task, TRUE); > - g_clear_object (&self->priv->task); > + port_probe_task_return_boolean (self, TRUE); > return; > } > > -- > 2.8.0 _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel