Update to 2.65.0
This commit is contained in:
parent
d43d0ce458
commit
46f4926d2c
@ -1,220 +0,0 @@
|
|||||||
From e4a690f5dd959e74b2d6054826f61509892c8aa7 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Philip Withnall <withnall@endlessm.com>
|
|
||||||
Date: Fri, 21 Feb 2020 14:44:44 +0000
|
|
||||||
Subject: [PATCH] gcancellable: Fix minor race between GCancellable and
|
|
||||||
GCancellableSource
|
|
||||||
MIME-Version: 1.0
|
|
||||||
Content-Type: text/plain; charset=UTF-8
|
|
||||||
Content-Transfer-Encoding: 8bit
|
|
||||||
|
|
||||||
There’s a minor race condition between cancellation of a `GCancellable`,
|
|
||||||
and disposal/finalisation of a `GCancellableSource` in another thread.
|
|
||||||
|
|
||||||
Thread A Thread B
|
|
||||||
g_cancellable_cancel(C)
|
|
||||||
→cancellable_source_cancelled(C, S)
|
|
||||||
g_source_unref(S)
|
|
||||||
cancellable_source_dispose(S)
|
|
||||||
→→g_source_ref(S)
|
|
||||||
→→# S is invalid at this point; crash
|
|
||||||
|
|
||||||
Thankfully, the `GCancellable` sets `cancelled_running` while it’s
|
|
||||||
emitting the `cancelled` signal, so if `cancellable_source_dispose()` is
|
|
||||||
called while that’s high, we know that the thread which is doing the
|
|
||||||
cancellation has already started (or is committed to starting) calling
|
|
||||||
`cancellable_source_cancelled()`.
|
|
||||||
|
|
||||||
Fix the race by resurrecting the `GCancellableSource` in
|
|
||||||
`cancellable_source_dispose()`, and signalling this using
|
|
||||||
`GCancellableSource.resurrected_during_cancellation`. Check for that
|
|
||||||
flag in `cancellable_source_cancelled()` and ignore cancellation if it’s
|
|
||||||
set.
|
|
||||||
|
|
||||||
The modifications to `resurrected_during_cancellation` and the
|
|
||||||
cancellable source’s refcount have to be done with `cancellable_mutex`
|
|
||||||
held so that they are seen atomically by each thread. This should not
|
|
||||||
affect performance too much, as it only happens during cancellation or
|
|
||||||
disposal of a `GCancellableSource`.
|
|
||||||
|
|
||||||
Signed-off-by: Philip Withnall <withnall@endlessm.com>
|
|
||||||
|
|
||||||
Fixes: #1841
|
|
||||||
---
|
|
||||||
gio/gcancellable.c | 43 +++++++++++++++++++++++
|
|
||||||
gio/tests/cancellable.c | 77 +++++++++++++++++++++++++++++++++++++++++
|
|
||||||
2 files changed, 120 insertions(+)
|
|
||||||
|
|
||||||
diff --git a/gio/gcancellable.c b/gio/gcancellable.c
|
|
||||||
index d9e58b8e8..e687cca23 100644
|
|
||||||
--- a/gio/gcancellable.c
|
|
||||||
+++ b/gio/gcancellable.c
|
|
||||||
@@ -643,6 +643,8 @@ typedef struct {
|
|
||||||
|
|
||||||
GCancellable *cancellable;
|
|
||||||
gulong cancelled_handler;
|
|
||||||
+ /* Protected by cancellable_mutex: */
|
|
||||||
+ gboolean resurrected_during_cancellation;
|
|
||||||
} GCancellableSource;
|
|
||||||
|
|
||||||
/*
|
|
||||||
@@ -661,8 +663,24 @@ cancellable_source_cancelled (GCancellable *cancellable,
|
|
||||||
gpointer user_data)
|
|
||||||
{
|
|
||||||
GSource *source = user_data;
|
|
||||||
+ GCancellableSource *cancellable_source = (GCancellableSource *) source;
|
|
||||||
+
|
|
||||||
+ g_mutex_lock (&cancellable_mutex);
|
|
||||||
+
|
|
||||||
+ /* Drop the reference added in cancellable_source_dispose(); see the comment there.
|
|
||||||
+ * The reference must be dropped after unlocking @cancellable_mutex since
|
|
||||||
+ * it could be the final reference, and the dispose function takes
|
|
||||||
+ * @cancellable_mutex. */
|
|
||||||
+ if (cancellable_source->resurrected_during_cancellation)
|
|
||||||
+ {
|
|
||||||
+ cancellable_source->resurrected_during_cancellation = FALSE;
|
|
||||||
+ g_mutex_unlock (&cancellable_mutex);
|
|
||||||
+ g_source_unref (source);
|
|
||||||
+ return;
|
|
||||||
+ }
|
|
||||||
|
|
||||||
g_source_ref (source);
|
|
||||||
+ g_mutex_unlock (&cancellable_mutex);
|
|
||||||
g_source_set_ready_time (source, 0);
|
|
||||||
g_source_unref (source);
|
|
||||||
}
|
|
||||||
@@ -684,12 +702,37 @@ cancellable_source_dispose (GSource *source)
|
|
||||||
{
|
|
||||||
GCancellableSource *cancellable_source = (GCancellableSource *)source;
|
|
||||||
|
|
||||||
+ g_mutex_lock (&cancellable_mutex);
|
|
||||||
+
|
|
||||||
if (cancellable_source->cancellable)
|
|
||||||
{
|
|
||||||
+ if (cancellable_source->cancellable->priv->cancelled_running)
|
|
||||||
+ {
|
|
||||||
+ /* There can be a race here: if thread A has called
|
|
||||||
+ * g_cancellable_cancel() and has got as far as committing to call
|
|
||||||
+ * cancellable_source_cancelled(), then thread B drops the final
|
|
||||||
+ * ref on the GCancellableSource before g_source_ref() is called in
|
|
||||||
+ * cancellable_source_cancelled(), then cancellable_source_dispose()
|
|
||||||
+ * will run through and the GCancellableSource will be finalised
|
|
||||||
+ * before cancellable_source_cancelled() gets to g_source_ref(). It
|
|
||||||
+ * will then be left in a state where it’s committed to using a
|
|
||||||
+ * dangling GCancellableSource pointer.
|
|
||||||
+ *
|
|
||||||
+ * Eliminate that race by resurrecting the #GSource temporarily, and
|
|
||||||
+ * then dropping that reference in cancellable_source_cancelled(),
|
|
||||||
+ * which should be guaranteed to fire because we’re inside a
|
|
||||||
+ * @cancelled_running block.
|
|
||||||
+ */
|
|
||||||
+ g_source_ref (source);
|
|
||||||
+ cancellable_source->resurrected_during_cancellation = TRUE;
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
g_clear_signal_handler (&cancellable_source->cancelled_handler,
|
|
||||||
cancellable_source->cancellable);
|
|
||||||
g_clear_object (&cancellable_source->cancellable);
|
|
||||||
}
|
|
||||||
+
|
|
||||||
+ g_mutex_unlock (&cancellable_mutex);
|
|
||||||
}
|
|
||||||
|
|
||||||
static gboolean
|
|
||||||
diff --git a/gio/tests/cancellable.c b/gio/tests/cancellable.c
|
|
||||||
index 4ba9f6326..002bdccca 100644
|
|
||||||
--- a/gio/tests/cancellable.c
|
|
||||||
+++ b/gio/tests/cancellable.c
|
|
||||||
@@ -228,6 +228,82 @@ test_cancel_null (void)
|
|
||||||
g_cancellable_cancel (NULL);
|
|
||||||
}
|
|
||||||
|
|
||||||
+typedef struct
|
|
||||||
+{
|
|
||||||
+ GCond cond;
|
|
||||||
+ GMutex mutex;
|
|
||||||
+ GSource *cancellable_source; /* (owned) */
|
|
||||||
+} ThreadedDisposeData;
|
|
||||||
+
|
|
||||||
+static gboolean
|
|
||||||
+cancelled_cb (GCancellable *cancellable,
|
|
||||||
+ gpointer user_data)
|
|
||||||
+{
|
|
||||||
+ /* Nothing needs to be done here. */
|
|
||||||
+ return G_SOURCE_CONTINUE;
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
+static gpointer
|
|
||||||
+threaded_dispose_thread_cb (gpointer user_data)
|
|
||||||
+{
|
|
||||||
+ ThreadedDisposeData *data = user_data;
|
|
||||||
+
|
|
||||||
+ /* Synchronise with the main thread before trying to reproduce the race. */
|
|
||||||
+ g_mutex_lock (&data->mutex);
|
|
||||||
+ g_cond_broadcast (&data->cond);
|
|
||||||
+ g_mutex_unlock (&data->mutex);
|
|
||||||
+
|
|
||||||
+ /* Race with cancellation of the cancellable. */
|
|
||||||
+ g_source_unref (data->cancellable_source);
|
|
||||||
+
|
|
||||||
+ return NULL;
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
+static void
|
|
||||||
+test_cancellable_source_threaded_dispose (void)
|
|
||||||
+{
|
|
||||||
+ guint i;
|
|
||||||
+
|
|
||||||
+ g_test_summary ("Test a thread race between disposing of a GCancellableSource "
|
|
||||||
+ "(in one thread) and cancelling the GCancellable it refers "
|
|
||||||
+ "to (in another thread)");
|
|
||||||
+ g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1841");
|
|
||||||
+
|
|
||||||
+ for (i = 0; i < 100000; i++)
|
|
||||||
+ {
|
|
||||||
+ GCancellable *cancellable = NULL;
|
|
||||||
+ GSource *cancellable_source = NULL;
|
|
||||||
+ ThreadedDisposeData data;
|
|
||||||
+ GThread *thread = NULL;
|
|
||||||
+
|
|
||||||
+ /* Create a cancellable and a cancellable source for it. For this test,
|
|
||||||
+ * there’s no need to attach the source to a #GMainContext. */
|
|
||||||
+ cancellable = g_cancellable_new ();
|
|
||||||
+ cancellable_source = g_cancellable_source_new (cancellable);
|
|
||||||
+ g_source_set_callback (cancellable_source, G_SOURCE_FUNC (cancelled_cb), NULL, NULL);
|
|
||||||
+
|
|
||||||
+ /* Create a new thread and wait until it’s ready to execute before
|
|
||||||
+ * cancelling our cancellable. */
|
|
||||||
+ g_cond_init (&data.cond);
|
|
||||||
+ g_mutex_init (&data.mutex);
|
|
||||||
+ data.cancellable_source = g_steal_pointer (&cancellable_source);
|
|
||||||
+
|
|
||||||
+ g_mutex_lock (&data.mutex);
|
|
||||||
+ thread = g_thread_new ("/cancellable-source/threaded-dispose",
|
|
||||||
+ threaded_dispose_thread_cb, &data);
|
|
||||||
+ g_cond_wait (&data.cond, &data.mutex);
|
|
||||||
+ g_mutex_unlock (&data.mutex);
|
|
||||||
+
|
|
||||||
+ /* Race with disposal of the cancellable source. */
|
|
||||||
+ g_cancellable_cancel (cancellable);
|
|
||||||
+
|
|
||||||
+ g_thread_join (g_steal_pointer (&thread));
|
|
||||||
+ g_mutex_clear (&data.mutex);
|
|
||||||
+ g_cond_clear (&data.cond);
|
|
||||||
+ g_object_unref (cancellable);
|
|
||||||
+ }
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
int
|
|
||||||
main (int argc, char *argv[])
|
|
||||||
{
|
|
||||||
@@ -235,6 +311,7 @@ main (int argc, char *argv[])
|
|
||||||
|
|
||||||
g_test_add_func ("/cancellable/multiple-concurrent", test_cancel_multiple_concurrent);
|
|
||||||
g_test_add_func ("/cancellable/null", test_cancel_null);
|
|
||||||
+ g_test_add_func ("/cancellable-source/threaded-dispose", test_cancellable_source_threaded_dispose);
|
|
||||||
|
|
||||||
return g_test_run ();
|
|
||||||
}
|
|
||||||
--
|
|
||||||
2.24.1
|
|
||||||
|
|
11
glib2.spec
11
glib2.spec
@ -1,17 +1,13 @@
|
|||||||
%global _changelog_trimtime %(date +%s -d "1 year ago")
|
%global _changelog_trimtime %(date +%s -d "1 year ago")
|
||||||
|
|
||||||
Name: glib2
|
Name: glib2
|
||||||
Version: 2.64.3
|
Version: 2.65.0
|
||||||
Release: 1%{?dist}
|
Release: 1%{?dist}
|
||||||
Summary: A library of handy utility functions
|
Summary: A library of handy utility functions
|
||||||
|
|
||||||
License: LGPLv2+
|
License: LGPLv2+
|
||||||
URL: http://www.gtk.org
|
URL: http://www.gtk.org
|
||||||
Source0: http://download.gnome.org/sources/glib/2.64/glib-%{version}.tar.xz
|
Source0: http://download.gnome.org/sources/glib/2.65/glib-%{version}.tar.xz
|
||||||
|
|
||||||
# https://bugzilla.redhat.com/show_bug.cgi?id=1825230
|
|
||||||
# https://gitlab.gnome.org/GNOME/glib/-/commit/e4a690f5dd959e74b2d6054826f61509892c8aa7
|
|
||||||
Patch0: gcancellable-race-fix.patch
|
|
||||||
|
|
||||||
BuildRequires: chrpath
|
BuildRequires: chrpath
|
||||||
BuildRequires: gcc
|
BuildRequires: gcc
|
||||||
@ -219,6 +215,9 @@ glib-compile-schemas %{_datadir}/glib-2.0/schemas &> /dev/null || :
|
|||||||
%{_datadir}/installed-tests
|
%{_datadir}/installed-tests
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Mon Jun 22 2020 Kalev Lember <klember@redhat.com> - 2.65.0-1
|
||||||
|
- Update to 2.65.0
|
||||||
|
|
||||||
* Wed May 20 2020 Kalev Lember <klember@redhat.com> - 2.64.3-1
|
* Wed May 20 2020 Kalev Lember <klember@redhat.com> - 2.64.3-1
|
||||||
- Update to 2.64.3
|
- Update to 2.64.3
|
||||||
|
|
||||||
|
2
sources
2
sources
@ -1 +1 @@
|
|||||||
SHA512 (glib-2.64.3.tar.xz) = a3828c37a50e86eb8791be53bd8af848d144e4580841ffab28f3b6eae5144f5cdf4a5d4b43130615b97488e700b274c2468fc7d561b3701a1fc686349501a1db
|
SHA512 (glib-2.65.0.tar.xz) = 0bad09a41c7860d31522729e18a20d208de60935b081407cc9307cfc865b58e751ed3978b864081d4b59744c584d37e77cf0b055e9daae91cb73b4b93a6d9ee1
|
||||||
|
Loading…
Reference in New Issue
Block a user