[Emacs-diffs] master fe3188b: Fix crashes upon C-g on Posix TTY frames

[Eli Zaretskii]

branch: master
commit fe3188b1cecc7ac5534616c8edf14a84b1b3bbb0
Author: Eli Zaretskii <address@hidden>
Commit: Eli Zaretskii <address@hidden>

    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)
---
 src/keyboard.c  |    3 +++
 src/sysdep.c    |   17 +++++++++++++++++
 src/syssignal.h |    2 ++
 src/thread.c    |   27 +++++++++++++++++++++++++++
 src/thread.h    |    8 ++++++++
 5 files changed, 57 insertions(+)

diff --git a/src/keyboard.c b/src/keyboard.c
index 1fb1d49..f2ee313 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 3d2b9bd..96c9e53 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 3de83c7..62704fc 100644
--- a/src/syssignal.h
+++ b/src/syssignal.h
@@ -25,6 +25,8 @@ along with GNU Emacs.  If not, see 
<http://www.gnu.org/licenses/>.  */
 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 e8cb430..bf2cf1b 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -24,6 +24,7 @@ along with GNU Emacs.  If not, see 
<http://www.gnu.org/licenses/>.  */
 #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 e6084b1..7dee67d 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);