luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-02-23 10:42:11 +01:00
Code Issues Packages Projects Releases Wiki Activity

gmain: Use atomic logic to handle internal GSource flags

Browse Source 
We use flags in both locked paths and in public ones (to check if a
source is destroyed or running), but those checks are not using atomic
logic, thus they lead to races in various tests.

Fix them by atomically change and read the values.
And this fixes various tests in thread sanitizer.
This commit is contained in:
Marco Trevisan (Treviño) 2025-01-31 11:23:53 +01:00
parent 30fbe0e859
commit a5bc497021
2 changed files with 30 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
glib/gmain.c
View File

@ -307,8 +307,10 @@ typedef struct _GSourceIter
#define UNLOCK_CONTEXT(context) g_mutex_unlock (&context->mutex)
#define G_THREAD_SELF g_thread_self ()
#define SOURCE_DESTROYED(source) (((source)->flags & G_HOOK_FLAG_ACTIVE) == 0)
#define SOURCE_BLOCKED(source) (((source)->flags & G_SOURCE_BLOCKED) != 0)
#define SOURCE_DESTROYED(source) \
((g_atomic_int_get (&((source)->flags)) & G_HOOK_FLAG_ACTIVE) == 0)
#define SOURCE_BLOCKED(source) \
((g_atomic_int_get (&((source)->flags)) & G_SOURCE_BLOCKED) != 0)
/* Forward declarations */
@ -934,7 +936,7 @@ g_source_new (GSourceFuncs *source_funcs,
source->priority = G_PRIORITY_DEFAULT;
source->flags = G_HOOK_FLAG_ACTIVE;
g_atomic_int_set (&source->flags, G_HOOK_FLAG_ACTIVE);
source->priv->ready_time = -1;
@ -1284,8 +1286,8 @@ g_source_destroy_internal (GSource *source,
GSList *tmp_list;
gpointer old_cb_data;
GSourceCallbackFuncs *old_cb_funcs;
source->flags &= ~G_HOOK_FLAG_ACTIVE;
g_atomic_int_and (&source->flags, ~G_HOOK_FLAG_ACTIVE);
old_cb_data = source->callback_data;
old_cb_funcs = source->callback_funcs;
@ -1359,7 +1361,7 @@ g_source_destroy (GSource *source)
if (context)
g_source_destroy_internal (source, context, FALSE);
else
source->flags &= ~G_HOOK_FLAG_ACTIVE;
g_atomic_int_and (&source->flags, ~G_HOOK_FLAG_ACTIVE);
}
/**
@ -2007,9 +2009,9 @@ g_source_set_can_recurse (GSource *source,
LOCK_CONTEXT (context);
if (can_recurse)
source->flags |= G_SOURCE_CAN_RECURSE;
g_atomic_int_or (&source->flags, G_SOURCE_CAN_RECURSE);
else
source->flags &= ~G_SOURCE_CAN_RECURSE;
g_atomic_int_and (&source->flags, ~G_SOURCE_CAN_RECURSE);
if (context)
UNLOCK_CONTEXT (context);
@ -2029,8 +2031,8 @@ g_source_get_can_recurse (GSource *source)
{
g_return_val_if_fail (source != NULL, FALSE);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, FALSE);
return (source->flags & G_SOURCE_CAN_RECURSE) != 0;
return (g_atomic_int_get (&source->flags) & G_SOURCE_CAN_RECURSE) != 0;
}
static void
@ -3267,7 +3269,7 @@ block_source (GSource *source)
g_return_if_fail (!SOURCE_BLOCKED (source));
source->flags |= G_SOURCE_BLOCKED;
g_atomic_int_or (&source->flags, G_SOURCE_BLOCKED);
if (source->context)
{
@ -3301,8 +3303,8 @@ unblock_source (GSource *source)
g_return_if_fail (SOURCE_BLOCKED (source)); /* Source already unblocked */
g_return_if_fail (!SOURCE_DESTROYED (source));
source->flags &= ~G_SOURCE_BLOCKED;
g_atomic_int_and (&source->flags, ~G_SOURCE_BLOCKED);
tmp_list = source->poll_fds;
while (tmp_list)
@ -3339,7 +3341,7 @@ g_main_dispatch (GMainContext *context)
context->pending_dispatches->pdata[i] = NULL;
g_assert (source);
source->flags &= ~G_SOURCE_READY;
g_atomic_int_and (&source->flags, ~G_SOURCE_READY);
if (!SOURCE_DESTROYED (source))
{
@ -3363,11 +3365,12 @@ g_main_dispatch (GMainContext *context)
if (cb_funcs)
cb_funcs->ref (cb_data);
if ((source->flags & G_SOURCE_CAN_RECURSE) == 0)
if ((g_atomic_int_get (&source->flags) & G_SOURCE_CAN_RECURSE) == 0)
block_source (source);
was_in_call = source->flags & G_HOOK_FLAG_IN_CALL;
source->flags |= G_HOOK_FLAG_IN_CALL;
was_in_call = g_atomic_int_or (&source->flags,
(GSourceFlags) G_HOOK_FLAG_IN_CALL) &
G_HOOK_FLAG_IN_CALL;
if (cb_funcs)
cb_funcs->get (cb_data, source, &callback, &user_data);
@ -3404,9 +3407,9 @@ g_main_dispatch (GMainContext *context)
LOCK_CONTEXT (context);
if (!was_in_call)
source->flags &= ~G_HOOK_FLAG_IN_CALL;
g_atomic_int_and (&source->flags, ~G_HOOK_FLAG_IN_CALL);
if (SOURCE_BLOCKED (source) && !SOURCE_DESTROYED (source))
if (SOURCE_BLOCKED (source) && !SOURCE_DESTROYED (source))
unblock_source (source);
/* Note: this depends on the fact that we can't switch
@ -3773,7 +3776,7 @@ g_main_context_prepare_unlocked (GMainContext *context,
if ((n_ready > 0) && (source->priority > current_priority))
break;
if (!(source->flags & G_SOURCE_READY))
if (!(g_atomic_int_get (&source->flags) & G_SOURCE_READY))
{
gboolean result;
gboolean (* prepare) (GSource *source,
@ -3834,13 +3837,13 @@ g_main_context_prepare_unlocked (GMainContext *context,
while (ready_source)
{
ready_source->flags |= G_SOURCE_READY;
g_atomic_int_or (&ready_source->flags, G_SOURCE_READY);
ready_source = ready_source->priv->parent_source;
}
}
}
if (source->flags & G_SOURCE_READY)
if (g_atomic_int_get (&source->flags) & G_SOURCE_READY)
{
n_ready++;
current_priority = source->priority;
@ -4106,8 +4109,8 @@ g_main_context_check_unlocked (GMainContext *context,
if ((n_ready > 0) && (source->priority > max_priority))
break;
if (!(source->flags & G_SOURCE_READY))
{
if (!(g_atomic_int_get (&source->flags) & G_SOURCE_READY))
{
gboolean result;
gboolean (* check) (GSource *source);
@ -4177,13 +4180,13 @@ g_main_context_check_unlocked (GMainContext *context,
while (ready_source)
{
ready_source->flags |= G_SOURCE_READY;
g_atomic_int_or (&ready_source->flags, G_SOURCE_READY);
ready_source = ready_source->priv->parent_source;
}
}
}
if (source->flags & G_SOURCE_READY)
if (g_atomic_int_get (&source->flags) & G_SOURCE_READY)
{
g_source_ref (source);
g_ptr_array_add (context->pending_dispatches, source);

2
glib/gmain.h
View File

@ -275,7 +275,7 @@ struct _GSource
GMainContext *context;
gint priority;
guint flags;
guint flags; /* (atomic) */
guint source_id;
GSList *poll_fds;