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 adding a new private flag (so that the public one can still
be used as it used to, without having to use atomic APIs there) that is
now used to 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 4a3fba2141
commit 497a8c93cb
1 changed files with 31 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
glib/gmain.c
View File

@ -290,6 +290,8 @@ struct _GSourcePrivate
*/ */
GSList *fds; GSList *fds;
GSourceFlags flags; /* atomic */
GSourceDisposeFunc dispose; GSourceDisposeFunc dispose;
gboolean static_name; gboolean static_name;
@ -307,8 +309,10 @@ typedef struct _GSourceIter
#define UNLOCK_CONTEXT(context) g_mutex_unlock (&context->mutex) #define UNLOCK_CONTEXT(context) g_mutex_unlock (&context->mutex)
#define G_THREAD_SELF g_thread_self () #define G_THREAD_SELF g_thread_self ()
#define SOURCE_DESTROYED(source) (((source)->flags & G_HOOK_FLAG_ACTIVE) == 0) #define SOURCE_DESTROYED(source) \
#define SOURCE_BLOCKED(source) (((source)->flags & G_SOURCE_BLOCKED) != 0) ((g_atomic_int_get (&((source)->priv->flags)) & G_HOOK_FLAG_ACTIVE) == 0)
#define SOURCE_BLOCKED(source) \
((g_atomic_int_get (&((source)->priv->flags)) & G_SOURCE_BLOCKED) != 0)
/* Forward declarations */ /* Forward declarations */
@ -934,7 +938,7 @@ g_source_new (GSourceFuncs *source_funcs,
source->priority = G_PRIORITY_DEFAULT; source->priority = G_PRIORITY_DEFAULT;
source->flags = G_HOOK_FLAG_ACTIVE; g_atomic_int_set (&source->priv->flags, G_HOOK_FLAG_ACTIVE);
source->priv->ready_time = -1; source->priv->ready_time = -1;
@ -1285,7 +1289,7 @@ g_source_destroy_internal (GSource *source,
gpointer old_cb_data; gpointer old_cb_data;
GSourceCallbackFuncs *old_cb_funcs; GSourceCallbackFuncs *old_cb_funcs;
source->flags &= ~G_HOOK_FLAG_ACTIVE; g_atomic_int_and (&source->priv->flags, ~G_HOOK_FLAG_ACTIVE);
old_cb_data = source->callback_data; old_cb_data = source->callback_data;
old_cb_funcs = source->callback_funcs; old_cb_funcs = source->callback_funcs;
@ -1359,7 +1363,7 @@ g_source_destroy (GSource *source)
if (context) if (context)
g_source_destroy_internal (source, context, FALSE); g_source_destroy_internal (source, context, FALSE);
else else
source->flags &= ~G_HOOK_FLAG_ACTIVE; g_atomic_int_and (&source->priv->flags, ~G_HOOK_FLAG_ACTIVE);
} }
/** /**
@ -2007,9 +2011,9 @@ g_source_set_can_recurse (GSource *source,
LOCK_CONTEXT (context); LOCK_CONTEXT (context);
if (can_recurse) if (can_recurse)
source->flags |= G_SOURCE_CAN_RECURSE; g_atomic_int_or (&source->priv->flags, G_SOURCE_CAN_RECURSE);
else else
source->flags &= ~G_SOURCE_CAN_RECURSE; g_atomic_int_and (&source->priv->flags, ~G_SOURCE_CAN_RECURSE);
if (context) if (context)
UNLOCK_CONTEXT (context); UNLOCK_CONTEXT (context);
@ -2030,7 +2034,7 @@ g_source_get_can_recurse (GSource *source)
g_return_val_if_fail (source != NULL, FALSE); g_return_val_if_fail (source != NULL, FALSE);
g_return_val_if_fail (g_atomic_int_get (&source->ref_count) > 0, 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->priv->flags) & G_SOURCE_CAN_RECURSE) != 0;
} }
static void static void
@ -3240,7 +3244,7 @@ block_source (GSource *source)
g_return_if_fail (!SOURCE_BLOCKED (source)); g_return_if_fail (!SOURCE_BLOCKED (source));
source->flags |= G_SOURCE_BLOCKED; g_atomic_int_or (&source->priv->flags, G_SOURCE_BLOCKED);
if (source->context) if (source->context)
{ {
@ -3275,7 +3279,7 @@ unblock_source (GSource *source)
g_return_if_fail (SOURCE_BLOCKED (source)); /* Source already unblocked */ g_return_if_fail (SOURCE_BLOCKED (source)); /* Source already unblocked */
g_return_if_fail (!SOURCE_DESTROYED (source)); g_return_if_fail (!SOURCE_DESTROYED (source));
source->flags &= ~G_SOURCE_BLOCKED; g_atomic_int_and (&source->priv->flags, ~G_SOURCE_BLOCKED);
tmp_list = source->poll_fds; tmp_list = source->poll_fds;
while (tmp_list) while (tmp_list)
@ -3312,7 +3316,7 @@ g_main_dispatch (GMainContext *context)
context->pending_dispatches->pdata[i] = NULL; context->pending_dispatches->pdata[i] = NULL;
g_assert (source); g_assert (source);
source->flags &= ~G_SOURCE_READY; g_atomic_int_and (&source->priv->flags, ~G_SOURCE_READY);
if (!SOURCE_DESTROYED (source)) if (!SOURCE_DESTROYED (source))
{ {
@ -3336,11 +3340,12 @@ g_main_dispatch (GMainContext *context)
if (cb_funcs) if (cb_funcs)
cb_funcs->ref (cb_data); cb_funcs->ref (cb_data);
if ((source->flags & G_SOURCE_CAN_RECURSE) == 0) if ((g_atomic_int_get (&source->priv->flags) & G_SOURCE_CAN_RECURSE) == 0)
block_source (source); block_source (source);
was_in_call = source->flags & G_HOOK_FLAG_IN_CALL; was_in_call = g_atomic_int_or (&source->priv->flags,
source->flags |= G_HOOK_FLAG_IN_CALL; (GSourceFlags) G_HOOK_FLAG_IN_CALL) &
G_HOOK_FLAG_IN_CALL;
if (cb_funcs) if (cb_funcs)
cb_funcs->get (cb_data, source, &callback, &user_data); cb_funcs->get (cb_data, source, &callback, &user_data);
@ -3377,9 +3382,9 @@ g_main_dispatch (GMainContext *context)
LOCK_CONTEXT (context); LOCK_CONTEXT (context);
if (!was_in_call) if (!was_in_call)
source->flags &= ~G_HOOK_FLAG_IN_CALL; g_atomic_int_and (&source->priv->flags, ~G_HOOK_FLAG_IN_CALL);
if (SOURCE_BLOCKED (source) && !SOURCE_DESTROYED (source)) if (SOURCE_BLOCKED (source) && !SOURCE_DESTROYED (source))
unblock_source (source); unblock_source (source);
/* Note: this depends on the fact that we can't switch /* Note: this depends on the fact that we can't switch
@ -3746,7 +3751,7 @@ g_main_context_prepare_unlocked (GMainContext *context,
if ((n_ready > 0) && (source->priority > current_priority)) if ((n_ready > 0) && (source->priority > current_priority))
break; break;
if (!(source->flags & G_SOURCE_READY)) if (!(g_atomic_int_get (&source->priv->flags) & G_SOURCE_READY))
{ {
gboolean result; gboolean result;
gboolean (* prepare) (GSource *source, gboolean (* prepare) (GSource *source,
@ -3807,13 +3812,13 @@ g_main_context_prepare_unlocked (GMainContext *context,
while (ready_source) while (ready_source)
{ {
ready_source->flags |= G_SOURCE_READY; g_atomic_int_or (&ready_source->priv->flags, G_SOURCE_READY);
ready_source = ready_source->priv->parent_source; ready_source = ready_source->priv->parent_source;
} }
} }
} }
if (source->flags & G_SOURCE_READY) if (g_atomic_int_get (&source->priv->flags) & G_SOURCE_READY)
{ {
n_ready++; n_ready++;
current_priority = source->priority; current_priority = source->priority;
@ -4079,8 +4084,8 @@ g_main_context_check_unlocked (GMainContext *context,
if ((n_ready > 0) && (source->priority > max_priority)) if ((n_ready > 0) && (source->priority > max_priority))
break; break;
if (!(source->flags & G_SOURCE_READY)) if (!(g_atomic_int_get (&source->priv->flags) & G_SOURCE_READY))
{ {
gboolean result; gboolean result;
gboolean (* check) (GSource *source); gboolean (* check) (GSource *source);
@ -4150,13 +4155,13 @@ g_main_context_check_unlocked (GMainContext *context,
while (ready_source) while (ready_source)
{ {
ready_source->flags |= G_SOURCE_READY; g_atomic_int_or (&ready_source->priv->flags, G_SOURCE_READY);
ready_source = ready_source->priv->parent_source; ready_source = ready_source->priv->parent_source;
} }
} }
} }
if (source->flags & G_SOURCE_READY) if (g_atomic_int_get (&source->priv->flags) & G_SOURCE_READY)
{ {
g_source_ref (source); g_source_ref (source);
g_ptr_array_add (context->pending_dispatches, source); g_ptr_array_add (context->pending_dispatches, source);