[Emacs-diffs] master 5fefaaa: Fix a bug with signaling a thread that waits for condvar

[Eli Zaretskii]

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

    Fix a bug with signaling a thread that waits for condvar
    
    * src/thread.c (lisp_mutex_lock_for_thread): New function,
    with all the guts of lisp_mutex_lock.
    (lisp_mutex_lock): Call lisp_mutex_lock_for_thread.
    (condition_wait_callback): Don't call post_acquire_global_lock
    before locking the mutex, as that could cause a signaled thread to
    exit prematurely, because the condvar's mutex is recorded to be
    not owned by any thread, and with-mutex wants to unlock it as part
    of unwinding the stack in response to the signal.
---
 src/thread.c |   41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/src/thread.c b/src/thread.c
index 6048516..9ea7e12 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -128,11 +128,11 @@ lisp_mutex_init (lisp_mutex_t *mutex)
   sys_cond_init (&mutex->condition);
 }
 
-/* Lock MUTEX setting its count to COUNT, if non-zero, or to 1
-   otherwise.
+/* Lock MUTEX for thread LOCKER, setting its lock count to COUNT, if
+   non-zero, or to 1 otherwise.
 
-   If MUTEX is locked by the current thread, COUNT must be zero, and
-   the MUTEX's lock count will be incremented.
+   If MUTEX is locked by LOCKER, COUNT must be zero, and the MUTEX's
+   lock count will be incremented.
 
    If MUTEX is locked by another thread, this function will release
    the global lock, giving other threads a chance to run, and will
@@ -143,24 +143,25 @@ lisp_mutex_init (lisp_mutex_t *mutex)
    unlocked (meaning other threads could have run during the wait),
    zero otherwise.  */
 static int
-lisp_mutex_lock (lisp_mutex_t *mutex, int new_count)
+lisp_mutex_lock_for_thread (lisp_mutex_t *mutex, struct thread_state *locker,
+                           int new_count)
 {
   struct thread_state *self;
 
   if (mutex->owner == NULL)
     {
-      mutex->owner = current_thread;
+      mutex->owner = locker;
       mutex->count = new_count == 0 ? 1 : new_count;
       return 0;
     }
-  if (mutex->owner == current_thread)
+  if (mutex->owner == locker)
     {
       eassert (new_count == 0);
       ++mutex->count;
       return 0;
     }
 
-  self = current_thread;
+  self = locker;
   self->wait_condvar = &mutex->condition;
   while (mutex->owner != NULL && (new_count != 0
                                  || NILP (self->error_symbol)))
@@ -176,6 +177,12 @@ lisp_mutex_lock (lisp_mutex_t *mutex, int new_count)
   return 1;
 }
 
+static int
+lisp_mutex_lock (lisp_mutex_t *mutex, int new_count)
+{
+  return lisp_mutex_lock_for_thread (mutex, current_thread, new_count);
+}
+
 /* Decrement MUTEX's lock count.  If the lock count becomes zero after
    decrementing it, meaning the mutex is now unlocked, broadcast that
    to all the threads that might be waiting to lock the mutex.  This
@@ -398,16 +405,16 @@ condition_wait_callback (void *arg)
       self->wait_condvar = NULL;
     }
   self->event_object = Qnil;
-  /* Since sys_cond_wait could switch threads, we need to re-establish
-     ourselves as the current thread, otherwise lisp_mutex_lock will
-     record the wrong thread as the owner of the mutex lock.  */
-  post_acquire_global_lock (self);
-  /* Calling lisp_mutex_lock might yield to other threads while this
-     one waits for the mutex to become unlocked, so we need to
-     announce us as the current thread by calling
+  /* Since sys_cond_wait could switch threads, we need to lock the
+     mutex for the thread which was the current when we were called,
+     otherwise lisp_mutex_lock will record the wrong thread as the
+     owner of the mutex lock.  */
+  lisp_mutex_lock_for_thread (&mutex->mutex, self, saved_count);
+  /* Calling lisp_mutex_lock_for_thread might yield to other threads
+     while this one waits for the mutex to become unlocked, so we need
+     to announce us as the current thread by calling
      post_acquire_global_lock.  */
-  if (lisp_mutex_lock (&mutex->mutex, saved_count))
-    post_acquire_global_lock (self);
+  post_acquire_global_lock (self);
 }
 
 DEFUN ("condition-wait", Fcondition_wait, Scondition_wait, 1, 1, 0,