GNOME Bugzilla – Bug 674747
Critical when cancelling messages from got-headers
Last modified: 2012-05-02 14:49:38 UTC
I was getting those by just logging in to GMail: (epiphany:10628): GLib-GIO-CRITICAL **: g_simple_async_result_take_error: assertion `G_IS_SIMPLE_ASYNC_RESULT (simple)' failed (epiphany:10628): GLib-GIO-CRITICAL **: g_simple_async_result_complete: assertion `G_IS_SIMPLE_ASYNC_RESULT (simple)' failed (epiphany:10628): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed With the new giobased stuff the following situation happens sometimes: 1- the client decides to cancel some message 2- send_request_finished() is called by the async session and... 3- send_request_return_result() clears the item->result and completes the async operation 4- right after that soup_message_io dispatches the source whose callback is read_ready_cb 5- that calls again send_request_return_result() that will trigger the above asserts as the result is NULL.
Something like --- a/libsoup/soup-session-async.c +++ b/libsoup/soup-session-async.c @@ -756,7 +756,9 @@ read_ready_cb (SoupMessage *msg, gpointer user_data) GError *error = NULL; if (g_cancellable_set_error_if_cancelled (item->cancellable, &error)) { - send_request_return_result (item, NULL, error); + /* Something else already took care of it. */ + if (item->result) + send_request_return_result (item, NULL, error); return FALSE; } looks like a valid approach, doesn't it?
Actually I am not very sure about that. First of all it does not fix all the issues with cancellations. The actual issue IMO is that we're not handling properly message cancellation. The user might call soup_session_cancel_message() or cancel the GCancellable passed as argument to a SoupRequest. The former does not update the GCancellable (not sure if it should do it or not) but the fact is that the current code does not check if the message is cancelled or not but just if the GCancellable was cancelled.
(In reply to comment #2) > Actually I am not very sure about that. First of all it does not fix all the > issues with cancellations. The actual issue IMO is that we're not handling > properly message cancellation. The user might call > soup_session_cancel_message() or cancel the GCancellable passed as argument to > a SoupRequest. The former does not update the GCancellable soup_session_send_request_async() replaces item->cancellable with the cancellable that was passed in to that method, and soup_session_cancel_message() cancels item->cancellable, so the cancellable should end up cancelled... Can you add a test case to requester-test that demonstrates the bug?
(In reply to comment #3) > (In reply to comment #2) > > Actually I am not very sure about that. First of all it does not fix all the > > issues with cancellations. The actual issue IMO is that we're not handling > > properly message cancellation. The user might call > > soup_session_cancel_message() or cancel the GCancellable passed as argument to > > a SoupRequest. The former does not update the GCancellable > > soup_session_send_request_async() replaces item->cancellable with the > cancellable that was passed in to that method, and > soup_session_cancel_message() cancels item->cancellable, so the cancellable > should end up cancelled... Ah indeed, I haven't seen the code in the parent class... > Can you add a test case to requester-test that demonstrates the bug? let's see what I could do...
Ok, so forget everything about message cancellations (I changed the bug title accordingly). It turned out that the problem happens when we receive a 403 Forbidden from a server. This is the backtrace of the CRITICAL
+ Trace 230147
so as you can see, there are a couple of issues here (correct me if I'm wrong because some of them can be designed like that): 1- the critical happens because there is no stream in the async result and there is also no error 2- send_request_return_result() assumes that either stream != NULL or error != NULL, but both are not NULL at the same time (something that happens here because in send_request_finished() there is no error and there is no mostream either so that will call send_request_return_result (item, NULL, NULL)) 3- MHO is that we should propagate that error In 2.38.1 for example, it was not happening because there was not a send_request_finished() call in the FINISHING item queue state. PS: I reproduced it just by typing gmail.com in epy with privoxy on (gmail requests some youtube URL that privoxy blocks accordingly) but I guess that any invalid https url could trigger the same
nope, you were right the first time :)
(In reply to comment #6) > nope, you were right the first time :) I think I'm talking about two different issues with the same outcome :)
The case I could reproduce is fixed in master. See if it works for you?
Working fine now. Thx for the fix!