Commit 5fefaaa8 authored by Eli Zaretskii's avatar Eli Zaretskii
Browse files

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.
parent 57153260
......@@ -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,
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment