From: Eli Zaretskii Date: Mon, 19 Dec 2016 17:11:16 +0000 (+0200) Subject: Fix crashes upon C-g on Posix TTY frames X-Git-Tag: emacs-26.0.90~1051 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=fe3188b1cecc7ac5534616c8edf14a84b1b3bbb0;p=emacs.git Fix crashes upon C-g on Posix TTY frames * src/thread.h (struct thread_state): New member not_holding_lock. (maybe_reacquire_global_lock): Add prototype. * src/thread.c: Include syssignal.h. (maybe_reacquire_global_lock): New function. (really_call_select): Set the not_holding_lock member of the thread state before releasing the lock, and rest it after re-acquiring the lock when the select function returns. Block SIGINT while doing this to make sure we are not interrupted on TTY frames. * src/sysdep.c (block_interrupt_signal, restore_signal_mask): New functions. * src/syssignal.h (block_interrupt_signal, restore_signal_mask): Add prototypes. * src/keyboard.c (read_char) [THREADS_ENABLED]: Call maybe_reacquire_global_lock. (Bug#25178) --- diff --git a/src/keyboard.c b/src/keyboard.c index 1fb1d492ce6..f2ee313b8c9 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -2571,6 +2571,9 @@ read_char (int commandflag, Lisp_Object map, so restore it now. */ restore_getcjmp (save_jump); pthread_sigmask (SIG_SETMASK, &empty_mask, 0); +#if THREADS_ENABLED + maybe_reacquire_global_lock (); +#endif unbind_to (jmpcount, Qnil); XSETINT (c, quit_char); internal_last_event_frame = selected_frame; diff --git a/src/sysdep.c b/src/sysdep.c index 3d2b9bdeeee..96c9e538409 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -765,6 +765,23 @@ unblock_child_signal (sigset_t const *oldset) pthread_sigmask (SIG_SETMASK, oldset, 0); } +/* Block SIGINT. */ +void +block_interrupt_signal (sigset_t *oldset) +{ + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, SIGINT); + pthread_sigmask (SIG_BLOCK, &blocked, oldset); +} + +/* Restore previously saved signal mask. */ +void +restore_signal_mask (sigset_t const *oldset) +{ + pthread_sigmask (SIG_SETMASK, oldset, 0); +} + #endif /* !MSDOS */ /* Saving and restoring the process group of Emacs's terminal. */ diff --git a/src/syssignal.h b/src/syssignal.h index 3de83c71759..62704fc351e 100644 --- a/src/syssignal.h +++ b/src/syssignal.h @@ -25,6 +25,8 @@ along with GNU Emacs. If not, see . */ extern void init_signals (bool); extern void block_child_signal (sigset_t *); extern void unblock_child_signal (sigset_t const *); +extern void block_interrupt_signal (sigset_t *); +extern void restore_signal_mask (sigset_t const *); extern void block_tty_out_signal (sigset_t *); extern void unblock_tty_out_signal (sigset_t const *); diff --git a/src/thread.c b/src/thread.c index e8cb430119f..bf2cf1b06c8 100644 --- a/src/thread.c +++ b/src/thread.c @@ -24,6 +24,7 @@ along with GNU Emacs. If not, see . */ #include "buffer.h" #include "process.h" #include "coding.h" +#include "syssignal.h" static struct thread_state primary_thread; @@ -100,6 +101,23 @@ acquire_global_lock (struct thread_state *self) post_acquire_global_lock (self); } +/* This is called from keyboard.c when it detects that SIGINT + interrupted thread_select before the current thread could acquire + the lock. We must acquire the lock to prevent a thread from + running without holding the global lock, and to avoid repeated + calls to sys_mutex_unlock, which invokes undefined behavior. */ +void +maybe_reacquire_global_lock (void) +{ + if (current_thread->not_holding_lock) + { + struct thread_state *self = current_thread; + + acquire_global_lock (self); + current_thread->not_holding_lock = 0; + } +} + static void @@ -493,11 +511,20 @@ really_call_select (void *arg) { struct select_args *sa = arg; struct thread_state *self = current_thread; + sigset_t oldset; + block_interrupt_signal (&oldset); + self->not_holding_lock = 1; release_global_lock (); + restore_signal_mask (&oldset); + sa->result = (sa->func) (sa->max_fds, sa->rfds, sa->wfds, sa->efds, sa->timeout, sa->sigmask); + + block_interrupt_signal (&oldset); acquire_global_lock (self); + self->not_holding_lock = 0; + restore_signal_mask (&oldset); } int diff --git a/src/thread.h b/src/thread.h index e6084b13c22..7dee67d6595 100644 --- a/src/thread.h +++ b/src/thread.h @@ -171,6 +171,13 @@ struct thread_state interrupter should broadcast to this condition. */ sys_cond_t *wait_condvar; + /* This thread might have released the global lock. If so, this is + non-zero. When a thread runs outside thread_select with this + flag non-zero, it means it has been interrupted by SIGINT while + in thread_select, and didn't have a chance of acquiring the lock. + It must do so ASAP. */ + int not_holding_lock; + /* Threads are kept on a linked list. */ struct thread_state *next_thread; }; @@ -224,6 +231,7 @@ extern void unmark_threads (void); extern void finalize_one_thread (struct thread_state *state); extern void finalize_one_mutex (struct Lisp_Mutex *); extern void finalize_one_condvar (struct Lisp_CondVar *); +extern void maybe_reacquire_global_lock (void); extern void init_threads_once (void); extern void init_threads (void);