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 642573 - Race in SoupAsyncSession handling of idle_run_queue_source
Race in SoupAsyncSession handling of idle_run_queue_source
Status: RESOLVED NOTABUG
Product: libsoup
Classification: Core
Component: API
2.32.x
Other Linux
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2011-02-17 14:06 UTC by David Woodhouse
Modified: 2012-07-19 13:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix (1.68 KB, application/octet-stream)
2011-02-17 14:06 UTC, David Woodhouse
Details

Description David Woodhouse 2011-02-17 14:06:01 UTC
Created attachment 181130 [details]
fix

When I call soup_session_queue_request() from a thread other than the one running the mainloop, request completion sometimes fails to occur and my application (evolution-ews) gets stuck.

The problem is caused by a race in handling of priv->idle_run_queue_source, between the idle_run_queue() function in soup-session-async.c, and the do_idle_run_queue() function that triggers it to be run. The latter ends up *not* registering a new idle source to call idle_run_queue(), because it mistakenly thinks that there's one already registered. 

The attached patch adds locking, and fixes the problem.

I suppose it's vaguely possible that the answer could be "Don't Do That Then", and that we should not be calling soup_session_queue_request() from any other thread; that we should do something awful like registering another idle callback to do it for us from the mainloop thread. I hope not, because that would put libsoup firmly in the category of "horrid things I never want to have to use in a threaded environment", like gconf.
Comment 1 Dan Winship 2011-02-17 14:28:06 UTC
"Don't Do That Then". SoupSessionAsync is explicitly declared to be non-thread-safe, and you will certainly run into more problems with it later even with this patch. (Mostly of the "completion callback gets run in another thread before the starting method returns to its caller; mayhem ensues" sort.)

If you need thread-safety, use SoupSessionSync, which *is* thread-safe. (The "Async" and "Sync" in the names is not quite right; SoupSessionAsync is for non-threaded use via a GMainLoop, and SoupSessionSync is for blocking use from multiple threads. Although as a kludge, if you set the async-context property on SoupSessionSync, you can still use soup_session_queue_message() on it, from whatever thread you want, and it will just send the request off to another thread to run.)
Comment 2 Mikhail 2012-07-19 13:27:20 UTC
Why patch is rejected?
Comment 3 David Woodhouse 2012-07-19 13:37:59 UTC
Because libsoup (at least SoupSessionAsync) is *intentionally* not thread-safe. If we fix this one locking bug, apparently we'll find others too.

Personally, I'd much prefer to find and fix *all* the bugs and declare it thread safe. But it's not my project...