After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 683404 - G_CRITICALs and eventual crashes in try_run_until_read()
G_CRITICALs and eventual crashes in try_run_until_read()
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: HTTP Transport
unspecified
Other Linux
: Normal major
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2012-09-05 11:10 UTC by Sergio Villar
Modified: 2012-09-17 17:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Temptative fix (1.28 KB, patch)
2012-09-06 17:05 UTC, Sergio Villar
none Details | Review

Description Sergio Villar 2012-09-05 11:10:44 UTC
Sometimes after a message is canceled:

1- the msg queue item is removed from the queue
2- the msg queue item ref_count becomes 0
3- a source is dispatched and read_ready_cb() is called

So there are a couple of problems with that:

a) the msg queue item is freed so we're dealing with invalid memory
b) accessing item in try_run_until_read() might work but the function will always fail because there is nothing inside item (neither result, nor callback nor nothing)

I found a pretty reliable way to reproduce this:
1- goto http://wildammo.com/2012/06/15/kodak-moment-55-perfectly-timed-hilariously-epic-photos
2- repeatedly click on the right arrow to check the next photo without letting the photo load completely

BTW the criticals looks like:

GLib-GIO-CRITICAL **: g_simple_async_result_take_error: assertion `G_IS_SIMPLE_ASYNC_RESULT (simple)' failed

GLib-GIO-CRITICAL **: g_simple_async_result_complete: assertion `G_IS_SIMPLE_ASYNC_RESULT (simple)' failed

GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

and the crashes are caused by invalid accesses to the message queue item fields.
Comment 1 Sergio Villar 2012-09-06 16:33:37 UTC
After some bisecting it looks like that it all started with this 61b86e07
Comment 2 Dan Winship 2012-09-06 17:00:18 UTC
so the relevant part of that commit is presumably the part where we no longer leak SoupMessageQueueItems on redirects... must have been hiding a refcounting/cleanup bug elsewhere
Comment 3 Sergio Villar 2012-09-06 17:05:09 UTC
Created attachment 223671 [details] [review]
Temptative fix

so this partial revert nukes the CRITICALs
Comment 4 Dan Winship 2012-09-17 17:56:56 UTC
I don't know why your patch would have fixed things (or, for that
matter, why my valgrind fixes broke it), but I've committed another
fix that definitely does make sense (and fixes this bug).