cancellable: Don't assert if finalization races with cancellation

Commit 281e3010 narrowed the race between GCancellable::cancelled and
GCancellableSource's finalize(), but did not prevent it: there was
nothing to stop cancellation from occurring after the refcount drops
to 0, but before g_source_unref_internal() bumps it back up to 1 to
run finalize().

GCancellable cannot be expected to detect that situation, because the
only way it has to detect last-unref is finalize(), but in that
situation finalize() hasn't happened yet.

Instead of detecting last-unref, relax the precondition a little
to make it detect finalization: priv is only poisoned (set to NULL)
after the finalize() function has been called, so we can assume that
GCancellable has already seen finalize() by then.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=791754
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=884654
This commit is contained in:
Simon McVittie 2017-12-19 10:58:32 +00:00
parent a4686b8ea1
commit 7f3bfcb891
2 changed files with 21 additions and 1 deletions

View File

@ -644,6 +644,18 @@ typedef struct {
guint cancelled_handler;
} GCancellableSource;
/*
* We can't guarantee that the source still has references, so we are
* relying on the fact that g_source_set_ready_time() no longer makes
* assertions about the reference count - the source might be in the
* window between last-unref and finalize, during which its refcount
* is officially 0. However, we *can* guarantee that it's OK to
* dereference it in a limited way, because we know we haven't yet reached
* cancellable_source_finalize() - if we had, then we would have waited
* for signal emission to finish, then disconnected the signal handler
* under the lock.
* See https://bugzilla.gnome.org/show_bug.cgi?id=791754
*/
static void
cancellable_source_cancelled (GCancellable *cancellable,
gpointer user_data)

View File

@ -1846,7 +1846,15 @@ g_source_set_ready_time (GSource *source,
GMainContext *context;
g_return_if_fail (source != NULL);
g_return_if_fail (source->ref_count > 0);
/* We deliberately don't check for ref_count > 0 here, because that
* breaks cancellable_source_cancelled() in GCancellable: it has no
* way to find out that the last-unref has happened until the
* finalize() function is called, but that's too late, because the
* ref_count already has already reached 0 before that time.
* However, priv is only poisoned (set to NULL) after finalize(),
* so we can use this as a simple guard against use-after-free.
* See https://bugzilla.gnome.org/show_bug.cgi?id=791754 */
g_return_if_fail (source->priv != NULL);
context = source->context;