From a9dc553893dcd26f047afecc8dc84d30c9bdcde6 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 30 Nov 2016 22:56:54 +0100 Subject: [PATCH] Fix two wait-condition-variable race conditions * libguile/threads.c (timed_wait): When looping to reacquire mutex, check if mutex owner after dropping mutex to run asyncs when the reacquire is interrupted. Also for asyncs that interrupted the initial wait, just return #t directly, and allow the caller to loop. Fixes a deadlock in which a thread could have pending asyncs after dropping a mutex and which prevent it from going to wait on a cond, but then the broadcast comes while nobody is waiting and the mutex is dropped, then after reacquiring the mutex when we go to wait again, we wait forever. The correct thing to do is after reacquiring the mutex, to allow the application to check if waiting is appropriate. --- libguile/threads.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/libguile/threads.c b/libguile/threads.c index 31a8cd48e..28f6cf4ea 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -1316,13 +1316,8 @@ timed_wait (enum scm_mutex_kind kind, struct scm_mutex *m, struct scm_cond *c, /* We woke up for some reason. Reacquire the mutex before doing anything else. */ - if (scm_is_eq (m->owner, SCM_BOOL_F)) - { - m->owner = current_thread->handle; - scm_i_pthread_mutex_unlock (&m->lock); - } - else if (kind == SCM_MUTEX_RECURSIVE && - scm_is_eq (m->owner, current_thread->handle)) + if (kind == SCM_MUTEX_RECURSIVE && + scm_is_eq (m->owner, current_thread->handle)) { m->level++; scm_i_pthread_mutex_unlock (&m->lock); @@ -1330,6 +1325,12 @@ timed_wait (enum scm_mutex_kind kind, struct scm_mutex *m, struct scm_cond *c, else while (1) { + if (scm_is_eq (m->owner, SCM_BOOL_F)) + { + m->owner = current_thread->handle; + scm_i_pthread_mutex_unlock (&m->lock); + break; + } block_self (m->waiting, &m->lock, waittime); if (scm_is_eq (m->owner, SCM_BOOL_F)) { @@ -1348,11 +1349,8 @@ timed_wait (enum scm_mutex_kind kind, struct scm_mutex *m, struct scm_cond *c, else if (err == ETIMEDOUT) return SCM_BOOL_F; else if (err == EINTR) - { - scm_async_tick (); - scm_i_scm_pthread_mutex_lock (&m->lock); - continue; - } + /* Let caller run scm_async_tick() and loop. */ + return SCM_BOOL_T; else { /* Shouldn't happen. */