luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-03-28 02:20:04 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'cancellable-source-dispose-races' into 'main'

Browse Source 
gcancellable: Ignore cancelled callback on disposed source

Closes #3448

See merge request GNOME/glib!4206
This commit is contained in:
Philip Withnall 2025-02-04 14:40:56 +00:00
commit 97fdba0d2f
2 changed files with 32 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
gio/gcancellable.c
View File

@ -640,6 +640,8 @@ g_cancellable_disconnect (GCancellable *cancellable,
typedef struct { typedef struct {
GSource source; GSource source;
/* Atomic: */
GSource **self_ptr;
/* Atomic: */ /* Atomic: */
GCancellable *cancellable; GCancellable *cancellable;
gulong cancelled_handler; gulong cancelled_handler;
@ -662,10 +664,16 @@ static void
cancellable_source_cancelled (GCancellable *cancellable, cancellable_source_cancelled (GCancellable *cancellable,
gpointer user_data) gpointer user_data)
{ {
GSource *source = user_data; GSource *source = g_atomic_pointer_exchange ((GSource **) user_data, NULL);
GCancellableSource *cancellable_source = (GCancellableSource *) source; GCancellableSource *cancellable_source;
gboolean callback_was_not_called G_GNUC_UNUSED; gboolean callback_was_not_called G_GNUC_UNUSED;
/* The source is being disposed, so don't bother marking it as ready */
if (source == NULL)
return;
cancellable_source = (GCancellableSource *) source;
g_source_ref (source); g_source_ref (source);
g_source_set_ready_time (source, 0); g_source_set_ready_time (source, 0);
@ -688,7 +696,10 @@ cancellable_source_prepare (GSource *source,
cancellable = g_atomic_pointer_get (&cancellable_source->cancellable); cancellable = g_atomic_pointer_get (&cancellable_source->cancellable);
if (cancellable && !g_atomic_int_get (&cancellable->priv->cancelled_running)) if (cancellable && !g_atomic_int_get (&cancellable->priv->cancelled_running))
g_atomic_int_set (&cancellable_source->cancelled_callback_called, FALSE); {
g_atomic_int_set (&cancellable_source->cancelled_callback_called, FALSE);
g_atomic_pointer_set (cancellable_source->self_ptr, source);
}
return FALSE; return FALSE;
} }
@ -715,7 +726,10 @@ cancellable_source_dispose (GSource *source)
if (cancellable) if (cancellable)
{ {
if (g_atomic_int_get (&cancellable->priv->cancelled_running)) GSource *self_ptr =
g_atomic_pointer_exchange (cancellable_source->self_ptr, NULL);
if (self_ptr == NULL)
{ {
/* There can be a race here: if thread A has called /* There can be a race here: if thread A has called
* g_cancellable_cancel() and has got as far as committing to call * g_cancellable_cancel() and has got as far as committing to call
@ -810,14 +824,17 @@ g_cancellable_source_new (GCancellable *cancellable)
if (cancellable) if (cancellable)
{ {
cancellable_source->cancellable = g_object_ref (cancellable); cancellable_source->cancellable = g_object_ref (cancellable);
cancellable_source->self_ptr = g_new (GSource *, 1);
g_atomic_pointer_set (cancellable_source->self_ptr, source);
/* We intentionally don't use g_cancellable_connect() here, /* We intentionally don't use g_cancellable_connect() here,
* because we don't want the "at most once" behavior. * because we don't want the "at most once" behavior.
*/ */
cancellable_source->cancelled_handler = cancellable_source->cancelled_handler =
g_signal_connect (cancellable, "cancelled", g_signal_connect_data (cancellable, "cancelled",
G_CALLBACK (cancellable_source_cancelled), G_CALLBACK (cancellable_source_cancelled),
source); cancellable_source->self_ptr,
(GClosureNotify) g_free, G_CONNECT_DEFAULT);
if (g_cancellable_is_cancelled (cancellable)) if (g_cancellable_is_cancelled (cancellable))
g_source_set_ready_time (source, 0); g_source_set_ready_time (source, 0);
} }

16
gio/tests/cancellable.c
View File

@ -710,7 +710,7 @@ static void
on_racy_cancellable_cancelled (GCancellable *cancellable, on_racy_cancellable_cancelled (GCancellable *cancellable,
gpointer data) gpointer data)
{ {
gboolean *callback_called = data; gboolean *callback_called = data; /* (atomic) */
g_assert_true (g_cancellable_is_cancelled (cancellable)); g_assert_true (g_cancellable_is_cancelled (cancellable));
g_atomic_int_set (callback_called, TRUE); g_atomic_int_set (callback_called, TRUE);
@ -722,7 +722,7 @@ test_cancellable_cancel_reset_races (void)
GCancellable *cancellable; GCancellable *cancellable;
GThread *resetting_thread = NULL; GThread *resetting_thread = NULL;
GThread *cancelling_thread = NULL; GThread *cancelling_thread = NULL;
gboolean callback_called = FALSE; gboolean callback_called = FALSE; /* (atomic) */
g_test_summary ("Tests threads racing for cancelling and resetting a GCancellable"); g_test_summary ("Tests threads racing for cancelling and resetting a GCancellable");
@ -730,7 +730,7 @@ test_cancellable_cancel_reset_races (void)
g_cancellable_connect (cancellable, G_CALLBACK (on_racy_cancellable_cancelled), g_cancellable_connect (cancellable, G_CALLBACK (on_racy_cancellable_cancelled),
&callback_called, NULL); &callback_called, NULL);
g_assert_false (callback_called); g_assert_false (g_atomic_int_get (&callback_called));
resetting_thread = g_thread_new ("/cancellable/cancel-reset-races/resetting", resetting_thread = g_thread_new ("/cancellable/cancel-reset-races/resetting",
repeatedly_resetting_thread, repeatedly_resetting_thread,
@ -741,7 +741,7 @@ test_cancellable_cancel_reset_races (void)
g_thread_join (g_steal_pointer (&cancelling_thread)); g_thread_join (g_steal_pointer (&cancelling_thread));
g_thread_join (g_steal_pointer (&resetting_thread)); g_thread_join (g_steal_pointer (&resetting_thread));
g_assert_true (callback_called); g_assert_true (g_atomic_int_get (&callback_called));
g_object_unref (cancellable); g_object_unref (cancellable);
} }
@ -755,7 +755,7 @@ repeatedly_connecting_thread (gpointer data)
for (guint i = 0; i < iterations; ++i) for (guint i = 0; i < iterations; ++i)
{ {
gboolean callback_called = FALSE; gboolean callback_called = FALSE; /* (atomic) */
gboolean called; gboolean called;
gulong id = g_cancellable_connect (cancellable, gulong id = g_cancellable_connect (cancellable,
G_CALLBACK (on_racy_cancellable_cancelled), G_CALLBACK (on_racy_cancellable_cancelled),
@ -780,7 +780,7 @@ test_cancellable_cancel_reset_connect_races (void)
GThread *resetting_thread = NULL; GThread *resetting_thread = NULL;
GThread *cancelling_thread = NULL; GThread *cancelling_thread = NULL;
GThread *connecting_thread = NULL; GThread *connecting_thread = NULL;
gboolean callback_called = FALSE; gboolean callback_called = FALSE; /* (atomic) */
g_test_summary ("Tests threads racing for cancelling, connecting and disconnecting " g_test_summary ("Tests threads racing for cancelling, connecting and disconnecting "
" and resetting a GCancellable"); " and resetting a GCancellable");
@ -789,7 +789,7 @@ test_cancellable_cancel_reset_connect_races (void)
g_cancellable_connect (cancellable, G_CALLBACK (on_racy_cancellable_cancelled), g_cancellable_connect (cancellable, G_CALLBACK (on_racy_cancellable_cancelled),
&callback_called, NULL); &callback_called, NULL);
g_assert_false (callback_called); g_assert_false (g_atomic_int_get (&callback_called));
resetting_thread = g_thread_new ("/cancel-reset-connect-races/resetting", resetting_thread = g_thread_new ("/cancel-reset-connect-races/resetting",
repeatedly_resetting_thread, repeatedly_resetting_thread,
@ -803,7 +803,7 @@ test_cancellable_cancel_reset_connect_races (void)
g_thread_join (g_steal_pointer (&resetting_thread)); g_thread_join (g_steal_pointer (&resetting_thread));
g_thread_join (g_steal_pointer (&connecting_thread)); g_thread_join (g_steal_pointer (&connecting_thread));
g_assert_true (callback_called); g_assert_true (g_atomic_int_get (&callback_called));
g_object_unref (cancellable); g_object_unref (cancellable);
} }