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 711286 - g_cancellable_connect() doesn't work like its docs claim, has race condition
g_cancellable_connect() doesn't work like its docs claim, has race condition
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-11-02 13:57 UTC by Dan Winship
Modified: 2018-05-24 15:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gcancellable: don't use g_cancellable_connect() in GCancellableSource (1.54 KB, patch)
2013-11-02 14:00 UTC, Dan Winship
committed Details | Review

Description Dan Winship 2013-11-02 13:57:34 UTC
The g_cancellable_connect() docs claim:

 * @callback is called at most once, either directly at the
 * time of the connect if @cancellable is already cancelled,
 * or when @cancellable is cancelled in some thread.

but the "at most once" is only true if you throw the cancellable away after it fires. If you use the cancellable multiple times, then @callback will be called every time @cancellable is cancelled, UNLESS it was already cancelled when you called g_cancellable_connect(), in which case it it will only be called once.

Dropping the "at most once" rule and changing the already-cancelled case to connect to the signal anyway seems likely to break people, so the remaining options are (1) make the not-already-cancelled case also obey the "at most once" rule, by disconnecting from the signal handler after the first time it's emitted, or (2) document the inconsistency.

Either way, GCancellableSource is broken and needs to revert back to using g_signal_connect() for compatibility with its old behavior, qv bug 710691 comment 12.
Comment 1 Dan Winship 2013-11-02 14:00:01 UTC
Created attachment 258800 [details] [review]
gcancellable: don't use g_cancellable_connect() in GCancellableSource

g_cancellable_connect() is documented as calling its callback only
once, but GCancellableSource should trigger every time the cancellable
is cancelled.

https://bugzilla.gnome.org/show_bug.cgi?id=710691
Comment 2 Allison Karlitskaya (desrt) 2013-11-03 17:48:03 UTC
Can we deprecate resetting cancellables?
Comment 3 Dan Winship 2013-11-03 19:17:07 UTC
(In reply to comment #2)
> Can we deprecate resetting cancellables?

grepping over a jhbuild checkout shows g_cancellable_reset() being used in many modules.

GTlsConnectionGnutls is only using GCancellables here because GWakeup isn't public (bug 680102). And the use of cancellables makes the code confusing because the naming is wrong ("cancel" means "wake up", etc) so I'd be happy to change it if I had something to change it to.
Comment 4 Dan Winship 2013-11-11 14:29:28 UTC
Comment on attachment 258800 [details] [review]
gcancellable: don't use g_cancellable_connect() in GCancellableSource

committed this. (It's a regression even if we want to deprecate g_cancellable_reset())
Comment 5 GNOME Infrastructure Team 2018-05-24 15:47:57 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/774.