[Emacs-diffs] master 825f4dd: Avoid crashing if a new thread is signaled right away

[Eli Zaretskii]

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

    Avoid crashing if a new thread is signaled right away
    
    * src/thread.c (post_acquire_global_lock): Don't raise the pending
    signal if the thread's handlers were not yet set up, as that will
    cause Emacs to exit with a fatal error.  This can happen if a
    thread is signaled as soon as make-thread returns, before the new
    thread had an opportunity to acquire the global lock, set up the
    handlers, and call the thread function.
    
    * test/src/thread-tests.el (thread-signal-early): New test.
---
 src/thread.c             |   26 +++++++++++++++-----------
 test/src/thread-tests.el |    9 +++++++++
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/thread.c b/src/thread.c
index 6e9ca2e..e8cb430 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -77,7 +77,12 @@ post_acquire_global_lock (struct thread_state *self)
       set_buffer_internal_2 (current_buffer);
     }
 
-  if (!NILP (current_thread->error_symbol))
+   /* We could have been signaled while waiting to grab the global lock
+      for the first time since this thread was created, in which case
+      we didn't yet have the opportunity to set up the handlers.  Delay
+      raising the signal in that case (it will be actually raised when
+      the thread comes here after acquiring the lock the next time).  */
+  if (!NILP (current_thread->error_symbol) && handlerlist)
     {
       Lisp_Object sym = current_thread->error_symbol;
       Lisp_Object data = current_thread->error_data;
@@ -622,16 +627,15 @@ run_thread (void *state)
 
   acquire_global_lock (self);
 
-  { /* Put a dummy catcher at top-level so that handlerlist is never NULL.
-       This is important since handlerlist->nextfree holds the freelist
-       which would otherwise leak every time we unwind back to top-level.   */
-    handlerlist_sentinel = xzalloc (sizeof (struct handler));
-    handlerlist = handlerlist_sentinel->nextfree = handlerlist_sentinel;
-    struct handler *c = push_handler (Qunbound, CATCHER);
-    eassert (c == handlerlist_sentinel);
-    handlerlist_sentinel->nextfree = NULL;
-    handlerlist_sentinel->next = NULL;
-  }
+  /* Put a dummy catcher at top-level so that handlerlist is never NULL.
+     This is important since handlerlist->nextfree holds the freelist
+     which would otherwise leak every time we unwind back to top-level.   */
+  handlerlist_sentinel = xzalloc (sizeof (struct handler));
+  handlerlist = handlerlist_sentinel->nextfree = handlerlist_sentinel;
+  struct handler *c = push_handler (Qunbound, CATCHER);
+  eassert (c == handlerlist_sentinel);
+  handlerlist_sentinel->nextfree = NULL;
+  handlerlist_sentinel->next = NULL;
 
   /* It might be nice to do something with errors here.  */
   internal_condition_case (invoke_thread_function, Qt, do_nothing);
diff --git a/test/src/thread-tests.el b/test/src/thread-tests.el
index 7261cda..26c0b72 100644
--- a/test/src/thread-tests.el
+++ b/test/src/thread-tests.el
@@ -235,4 +235,13 @@
     (sit-for 1)
     (should (= (point) 21))))
 
+(ert-deftest thread-signal-early ()
+  "Test signaling a thread as soon as it is started by the OS."
+  (let ((thread
+         (make-thread #'(lambda ()
+                          (while t (thread-yield))))))
+    (thread-signal thread 'error nil)
+    (sit-for 1)
+    (should-not (thread-alive-p thread))))
+
 ;;; threads.el ends here