mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2024-11-10 19:36:18 +01:00
gmain: Access Unix signal handler state atomically
There are two variables which are used to pass state from the Unix signal handler interrupt function to the rest of `gmain.c`. They are currently defined as `sig_atomic_t`, which means that they are guaranteed to be interrupt safe. However, it does not guarantee they are thread-safe, and GLib attaches its signal handler interrupt function to a worker thread. Make them thread-safe using atomics. It’s not possible to use locks, as pthread mutex functions are not signal-handler-safe. In particular, this means we have to be careful not to end up using GLib’s fallback atomics implementation, as that secretly uses a mutex. Better to be unsafe than have a re-entrant call into `pthread_mutex_lock()` from a nested signal handler. This commit solves two problems: 1. Writes to `any_unix_signal_pending` and `unix_signal_pending` could be delivered out of order to the worker thread which calls `dispatch_unix_signals()`, resulting in signals not being handled until the next iteration of that worker thread. This is a performance problem but not a correctness problem. 2. Setting an element of `unix_signal_pending` from `g_unix_signal_handler()` and clearing it from `dispatch_unix_signals_unlocked()` (in the worker thread) could race, resulting in a signal emission being cleared without being handled. That’s a correctness problem. Signed-off-by: Philip Withnall <withnall@endlessm.com> Fixes: #1670
This commit is contained in:
parent
9792209f3c
commit
253f5cda82
22
glib/gmain.c
22
glib/gmain.c
@ -448,6 +448,10 @@ static GMainContext *glib_worker_context;
|
||||
|
||||
/* UNIX signals work by marking one of these variables then waking the
|
||||
* worker context to check on them and dispatch accordingly.
|
||||
*
|
||||
* Both variables must be accessed using atomic primitives, unless those atomic
|
||||
* primitives are implemented using fallback mutexes (as those aren’t safe in
|
||||
* an interrupt context).
|
||||
*/
|
||||
#ifdef HAVE_SIG_ATOMIC_T
|
||||
static volatile sig_atomic_t unix_signal_pending[NSIG];
|
||||
@ -5199,7 +5203,7 @@ dispatch_unix_signals_unlocked (void)
|
||||
gint i;
|
||||
|
||||
/* clear this first in case another one arrives while we're processing */
|
||||
any_unix_signal_pending = FALSE;
|
||||
g_atomic_int_set (&any_unix_signal_pending, 0);
|
||||
|
||||
/* We atomically test/clear the bit from the global array in case
|
||||
* other signals arrive while we are dispatching.
|
||||
@ -5218,9 +5222,7 @@ dispatch_unix_signals_unlocked (void)
|
||||
*
|
||||
* Note specifically: we must check _our_ array.
|
||||
*/
|
||||
pending[i] = unix_signal_pending[i];
|
||||
if (pending[i])
|
||||
unix_signal_pending[i] = FALSE;
|
||||
pending[i] = g_atomic_int_compare_and_exchange (&unix_signal_pending[i], 1, 0);
|
||||
}
|
||||
|
||||
/* handle GChildWatchSource instances */
|
||||
@ -5526,8 +5528,14 @@ g_unix_signal_handler (int signum)
|
||||
{
|
||||
gint saved_errno = errno;
|
||||
|
||||
unix_signal_pending[signum] = TRUE;
|
||||
any_unix_signal_pending = TRUE;
|
||||
#if defined(G_ATOMIC_LOCK_FREE) && defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4)
|
||||
g_atomic_int_set (&unix_signal_pending[signum], 1);
|
||||
g_atomic_int_set (&any_unix_signal_pending, 1);
|
||||
#else
|
||||
#warning "Can't use atomics in g_unix_signal_handler(): Unix signal handling will be racy"
|
||||
unix_signal_pending[signum] = 1;
|
||||
any_unix_signal_pending = 1;
|
||||
#endif
|
||||
|
||||
g_wakeup_signal (glib_worker_context->wakeup);
|
||||
|
||||
@ -5996,7 +6004,7 @@ glib_worker_main (gpointer data)
|
||||
g_main_context_iteration (glib_worker_context, TRUE);
|
||||
|
||||
#ifdef G_OS_UNIX
|
||||
if (any_unix_signal_pending)
|
||||
if (g_atomic_int_get (&any_unix_signal_pending))
|
||||
dispatch_unix_signals ();
|
||||
#endif
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user