From 825f4dd42f0f656bcb4536546b33fe8e54756468 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Mon, 12 Dec 2016 19:08:21 +0200 Subject: [PATCH] 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 6e9ca2e256b..e8cb430119f 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 7261cda9fbb..26c0b725ff8 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 -- 2.39.2