[Emacs-diffs] emacs-26 ea39d47: Avoid crashes on C-g when several threads wait for input

[Eli Zaretskii]

branch: emacs-26
commit ea39d470bf35e45f1d8e39795f06ac74b3c37fc7
Author: Eli Zaretskii <address@hidden>
Commit: Eli Zaretskii <address@hidden>

    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)
---
 src/keyboard.c | 14 +++++++-------
 src/lisp.h     | 39 ++++++++++++++++++++-------------------
 src/thread.c   | 16 +++++++++++-----
 src/thread.h   |  7 +++++++
 4 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index e8701b8..ee353d2 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 680c25d..bdb162a 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 42d7791..d075bdb 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 7fce867..cb2133d 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;