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 e8701b88708a3d7a750e2f7c0101faa611abd441..ee353d2b0785d8d6468134bb3f97847abc401caf 100644 (file)
--- 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 680c25d4c49b3c27e6dfdb89be42bddf13203558..bdb162aea4cecde0e48d2baf7c66ba0397e3bcd0 100644 (file)
--- 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)
-\f
-/* 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
 
 \f
 /* Elisp uses several stacks:

diff --git a/src/thread.c b/src/thread.c
index 42d7791ad0f56a51084af52c68cd903bdc45ff7b..d075bdb3a133683da8e5b15f20160f34158663cf 100644 (file)
--- 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 7fce8674f0e3c2838795d8c3f329f6287c0ad870..cb2133d72d41b34b17e9f54213c914b57c483908 100644 (file)
--- 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;