[Guile-commits] 01/01: Fix two wait-condition-variable race conditions

[Andy Wingo]

wingo pushed a commit to branch master
in repository guile.

commit a9dc553893dcd26f047afecc8dc84d30c9bdcde6
Author: Andy Wingo <address@hidden>
Date:   Wed Nov 30 22:56:54 2016 +0100

    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 31a8cd4..28f6cf4 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.  */