From: Eli Zaretskii Date: Wed, 4 Oct 2017 07:27:49 +0000 (+0300) Subject: Avoid crashes on C-g when several threads wait for input X-Git-Tag: emacs-26.0.90~52 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=ea39d470bf35e45f1d8e39795f06ac74b3c37fc7;p=emacs.git Avoid crashes on C-g when several threads wait for input * src/thread.h (m_getcjmp): New member of 'struct thread_state'. (getcjmp): Define to current thread's 'm_getcjmp'. * src/thread.c (maybe_reacquire_global_lock): Switch to main thread, since this is called from a SIGINT handler, which always runs in the context of the main thread. * src/lisp.h (sys_jmp_buf, sys_setjmp, sys_longjmp): Move the definitions before thread.h is included, as thread.h now uses sys_jmp_buf. * src/keyboard.c (getcjmp): Remove declaration. (read_char): Don't call maybe_reacquire_global_lock here. (handle_interrupt): Call maybe_reacquire_global_lock here, if invoked from the SIGINT handler, to make sure quit_throw_to_read_char runs with main thread's Lisp bindings and uses the main thread's jmp_buf buffer. (Bug#28630) --- diff --git a/src/keyboard.c b/src/keyboard.c index e8701b88708..ee353d2b078 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -145,10 +145,6 @@ static Lisp_Object recover_top_level_message; /* Message normally displayed by Vtop_level. */ static Lisp_Object regular_top_level_message; -/* For longjmp to where kbd input is being done. */ - -static sys_jmp_buf getcjmp; - /* True while displaying for echoing. Delays C-g throwing. */ static bool echoing; @@ -2570,9 +2566,6 @@ 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; @@ -10508,6 +10501,13 @@ handle_interrupt (bool in_signal_handler) outside of polling since we don't get SIGIO like X and we don't have a separate event loop thread like W32. */ #ifndef HAVE_NS +#ifdef THREADS_ENABLED + /* If we were called from a signal handler, we must be in the main + thread, see deliver_process_signal. So we must make sure the + main thread holds the global lock. */ + if (in_signal_handler) + maybe_reacquire_global_lock (); +#endif if (waiting_for_input && !echoing) quit_throw_to_read_char (in_signal_handler); #endif diff --git a/src/lisp.h b/src/lisp.h index 680c25d4c49..bdb162aea4c 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -1865,6 +1865,26 @@ verify (offsetof (struct Lisp_Sub_Char_Table, contents) == (offsetof (struct Lisp_Vector, contents) + SUB_CHAR_TABLE_OFFSET * sizeof (Lisp_Object))); + +/* Save and restore the instruction and environment pointers, + without affecting the signal mask. */ + +#ifdef HAVE__SETJMP +typedef jmp_buf sys_jmp_buf; +# define sys_setjmp(j) _setjmp (j) +# define sys_longjmp(j, v) _longjmp (j, v) +#elif defined HAVE_SIGSETJMP +typedef sigjmp_buf sys_jmp_buf; +# define sys_setjmp(j) sigsetjmp (j, 0) +# define sys_longjmp(j, v) siglongjmp (j, v) +#else +/* A platform that uses neither _longjmp nor siglongjmp; assume + longjmp does not affect the sigmask. */ +typedef jmp_buf sys_jmp_buf; +# define sys_setjmp(j) setjmp (j) +# define sys_longjmp(j, v) longjmp (j, v) +#endif + #include "thread.h" /*********************************************************************** @@ -3003,25 +3023,6 @@ extern void defvar_kboard (struct Lisp_Kboard_Objfwd *, const char *, int); static struct Lisp_Kboard_Objfwd ko_fwd; \ defvar_kboard (&ko_fwd, lname, offsetof (KBOARD, vname ## _)); \ } while (false) - -/* Save and restore the instruction and environment pointers, - without affecting the signal mask. */ - -#ifdef HAVE__SETJMP -typedef jmp_buf sys_jmp_buf; -# define sys_setjmp(j) _setjmp (j) -# define sys_longjmp(j, v) _longjmp (j, v) -#elif defined HAVE_SIGSETJMP -typedef sigjmp_buf sys_jmp_buf; -# define sys_setjmp(j) sigsetjmp (j, 0) -# define sys_longjmp(j, v) siglongjmp (j, v) -#else -/* A platform that uses neither _longjmp nor siglongjmp; assume - longjmp does not affect the sigmask. */ -typedef jmp_buf sys_jmp_buf; -# define sys_setjmp(j) setjmp (j) -# define sys_longjmp(j, v) longjmp (j, v) -#endif /* Elisp uses several stacks: diff --git a/src/thread.c b/src/thread.c index 42d7791ad0f..d075bdb3a13 100644 --- a/src/thread.c +++ b/src/thread.c @@ -101,14 +101,20 @@ 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. */ +/* This is called from keyboard.c when it detects that SIGINT was + delivered to the main thread and interrupted thread_select before + the main 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) { + /* SIGINT handler is always run on the main thread, see + deliver_process_signal, so reflect that in our thread-tracking + variables. */ + current_thread = &main_thread; + if (current_thread->not_holding_lock) { struct thread_state *self = current_thread; diff --git a/src/thread.h b/src/thread.h index 7fce8674f0e..cb2133d72d4 100644 --- a/src/thread.h +++ b/src/thread.h @@ -158,6 +158,13 @@ struct thread_state bool m_waiting_for_input; #define waiting_for_input (current_thread->m_waiting_for_input) + /* For longjmp to where kbd input is being done. This is per-thread + so that if more than one thread calls read_char, they don't + clobber each other's getcjmp, which will cause + quit_throw_to_read_char crash due to using a wrong stack. */ + sys_jmp_buf m_getcjmp; +#define getcjmp (current_thread->m_getcjmp) + /* The OS identifier for this thread. */ sys_thread_t thread_id;