<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 4, 2014 at 12:36 AM, Aleksander Morgado <span dir="ltr"><<a href="mailto:aleksander@aleksander.es" target="_blank">aleksander@aleksander.es</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hey,<br>
<br>
Good catch indeed, see a minor comment below.<br>
<div><div class="h5"><br>
On Tue, Feb 4, 2014 at 4:25 AM, Greg Suarez <<a href="mailto:gpsuarez2512@gmail.com">gpsuarez2512@gmail.com</a>> wrote:<br>
> When receiving multiple fragments device_store_transaction() is called<br>
> for each fragment received thus adding a timeout callback for each<br>
> fragment.  A timeout callback is only removed when all fragments are<br>
> received thus leaving dangling timeout callbacks.  This leads to a crash<br>
> when the MbimDevice is finalized.<br>
><br>
> This patch checks if the transaction has already registered a timeout<br>
> callback before adding a new one.<br>
><br>
> Signed-off-by: Greg Suarez <<a href="mailto:gpsuarez2512@gmail.com">gpsuarez2512@gmail.com</a>><br>
> ---<br>
>  src/libmbim-glib/mbim-device.c | 33 ++++++++++++++++++---------------<br>
>  1 file changed, 18 insertions(+), 15 deletions(-)<br>
><br>
> diff --git a/src/libmbim-glib/mbim-device.c b/src/libmbim-glib/mbim-device.c<br>
> index d8c6f81..ef47b7e 100644<br>
> --- a/src/libmbim-glib/mbim-device.c<br>
> +++ b/src/libmbim-glib/mbim-device.c<br>
> @@ -267,21 +267,24 @@ device_store_transaction (MbimDevice       *self,<br>
>      tr->wait_ctx->transaction_id = tr->transaction_id;<br>
>      tr->wait_ctx->type = type;<br>
><br>
> -    tr->timeout_id = g_timeout_add (timeout_ms,<br>
> -                                    (GSourceFunc)transaction_timed_out,<br>
> -                                    tr->wait_ctx);<br>
> -<br>
> -    if (tr->cancellable) {<br>
> -        tr->cancellable_id = g_cancellable_connect (tr->cancellable,<br>
> -                                                    (GCallback)transaction_cancelled,<br>
> -                                                    tr->wait_ctx,<br>
> -                                                    NULL);<br>
> -        if (!tr->cancellable_id) {<br>
> -            g_set_error_literal (error,<br>
> -                                 MBIM_CORE_ERROR,<br>
> -                                 MBIM_CORE_ERROR_ABORTED,<br>
> -                                 "Request is already cancelled");<br>
> -            return FALSE;<br>
> +    /* don't add timeout if one already exists */<br>
> +    if (!tr->timeout_id) {<br>
> +        tr->timeout_id = g_timeout_add (timeout_ms,<br>
> +                                        (GSourceFunc)transaction_timed_out,<br>
> +                                        tr->wait_ctx);<br>
> +<br>
<br>
</div></div>How about moving the if (tr->cancellable) change  out of the new if<br>
(!tr->timeout_id) you added, and also extend it so that it checks if<br>
(tr->cancellable && !tr->cancellable_id) ?<br>
<br>
Just to separate the logic of the timeout with that of the cancellable.<br>
<div class="HOEnZb"><div class="h5"><span class="HOEnZb"><font color="#888888"><br></font></span></div></div></blockquote><div><br></div><div>Sure, I'll resubmit the patches as a set.<br><br>Greg <br></div></div><br>
</div><div class="gmail_extra"><br></div></div>