GNOME Bugzilla – Bug 656387
GCancellable can be used concurrently
Last modified: 2011-08-19 09:27:58 UTC
I've had a really hard time tracking down why GCancellable can't be used concurrently (including asking Alex). I can't help but come to the conclusion that it can be used by multiple concurrent operations, contrary to the documentation. Will update the docs.
Previous discussion: http://www.mail-archive.com/gtk-devel-list@gnome.org/msg10628.html
Created attachment 193680 [details] [review] gio: GCancellable can be used concurrently * Update documentation to note that GCancellable can be used concurrently by multiple operations. * Add documentation to g_cancellable_reset that it will deadlock if called from within cancelled handler.
We should actually have a test for it if we're going to claim it works.
Sure. I'll include a test. But since this is threading, obviously most any test will prove that it *can* work (in the given circumstances), not that it *will* work (under any circumstances). Alex, if you have a moment to look over this as well, that would be perfect.
Created attachment 193697 [details] [review] gio: GCancellable can be used concurrently * Update documentation to note that GCancellable can be used concurrently by multiple operations. * Add documentation to g_cancellable_reset that it will deadlock if called from within cancelled handler. * Add test for multiple concurrent operations using the same cancellable.
The new test runs 45 different operations (randomly in threads or in mainloop) and then cancels them all, verifies that they all stopped short of their goal, and finished.
Comment on attachment 193697 [details] [review] gio: GCancellable can be used concurrently >+ * If this function is called from within a #GCancellable::cancelled >+ * then this will result in a deadlock. Missing the word "handler" or "callback". But if you call this while @cancellable is in use at all then there is a race condition, because if there are two ops using the cancellable, and handler A resets it, then handler B may or may not run, depending on thread scheduling. So, I think this should just say "if @cancellable is currently in use by a cancellable operation, then the result is undefined", which allows for both race conditions and deadlocks.
Yes, for sure calling g_cancellable_reset() while a cancellable is in use (by even one operation) is completely undefined and broken. In fact g_cancellable_reset() has such ugly semantics and effects, I really would prefer if it didn't exist at all. I can make that change, although the word 'deadlock' does really get people's attention.
Created attachment 193706 [details] [review] gio: GCancellable can be used concurrently * Update documentation to note that GCancellable can be used concurrently by multiple operations. * Add documentation to g_cancellable_reset that it will deadlock if called from within cancelled handler. * Add test for multiple concurrent operations using the same cancellable.
Review of attachment 193706 [details] [review]: > * Add documentation to g_cancellable_reset that it will deadlock > if called from within cancelled handler. Doesn't seem to be literally true. The added paragraph just speaks about undefined behaviour. Other than that, looks good to me.
Looks good to me too. please commit
Thanks. Merged. Changed the commit message as per Matthias' comment.