[Guile-commits] 03/05: Fix fat_mutex owner corruption and other problems in fat_mutex_unlock.

[Mark H. Weaver]

mhw pushed a commit to branch stable-2.0
in repository guile.

commit 1e86dc32a42af549fc9e4721ad48cdd7d296c042
Author: Mark H Weaver <address@hidden>
Date:   Fri Dec 18 02:29:48 2015 -0500

    Fix fat_mutex owner corruption and other problems in fat_mutex_unlock.
    
    Fixes <https://debbugs.gnu.org/22152>.
    Reported by Iwan Aucamp <address@hidden>.
    
    * libguile/threads.c (fat_mutex_unlock): Avoid repeatedly decrementing
      m->level and changing the owner of the mutex when 'block_self' returns
      EINTR.  Unblock asyncs before calling scm_syserror.  Refactor the code
      to clarify the EINTR loop, and to make it more apparent that the async
      blocks and unblocks are balanced.
---
 libguile/threads.c |   87 ++++++++++++++++++++++------------------------------
 1 file changed, 36 insertions(+), 51 deletions(-)

diff --git a/libguile/threads.c b/libguile/threads.c
index 6ae6818..dcbd24d 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -1607,70 +1607,55 @@ fat_mutex_unlock (SCM mutex, SCM cond,
        }
     }
 
+  if (m->level > 0)
+    m->level--;
+  if (m->level == 0)
+    {
+      /* Change the owner of MUTEX.  */
+      t->mutexes = scm_delq_x (mutex, t->mutexes);
+      m->owner = unblock_from_queue (m->waiting);
+    }
+
   if (! (SCM_UNBNDP (cond)))
     {
       c = SCM_CONDVAR_DATA (cond);
-      while (1)
-       {
-         int brk = 0;
-
-         if (m->level > 0)
-           m->level--;
-         if (m->level == 0)
-           {
-             /* Change the owner of MUTEX.  */
-             t->mutexes = scm_delq_x (mutex, t->mutexes);
-             m->owner = unblock_from_queue (m->waiting);
-           }
-
-         t->block_asyncs++;
+      t->block_asyncs++;
 
+      do
+       {
          err = block_self (c->waiting, cond, &m->lock, waittime);
          scm_i_pthread_mutex_unlock (&m->lock);
 
-         if (err == 0)
-           {
-             ret = 1;
-             brk = 1;
-           }
-         else if (err == ETIMEDOUT)
-           {
-             ret = 0;
-             brk = 1;
-           }
-         else if (err != EINTR)
-           {
-             errno = err;
-             scm_syserror (NULL);
-           }
-
-         if (brk)
-           {
-             if (relock)
-               scm_lock_mutex_timed (mutex, SCM_UNDEFINED, owner);
-             t->block_asyncs--;
-             break;
-           }
-
-         t->block_asyncs--;
-         scm_async_click ();
+          if (err == EINTR)
+            {
+              t->block_asyncs--;
+              scm_async_click ();
 
-         scm_remember_upto_here_2 (cond, mutex);
+              scm_remember_upto_here_2 (cond, mutex);
 
-         scm_i_scm_pthread_mutex_lock (&m->lock);
+              scm_i_scm_pthread_mutex_lock (&m->lock);
+              t->block_asyncs++;
+            }
        }
+      while (err == EINTR);
+
+      if (err == 0)
+        ret = 1;
+      else if (err == ETIMEDOUT)
+        ret = 0;
+      else
+        {
+          t->block_asyncs--;
+          errno = err;
+          scm_syserror (NULL);
+        }
+
+      if (relock)
+        scm_lock_mutex_timed (mutex, SCM_UNDEFINED, owner);
+      t->block_asyncs--;
     }
   else
     {
-      if (m->level > 0)
-       m->level--;
-      if (m->level == 0)
-       {
-         /* Change the owner of MUTEX.  */
-         t->mutexes = scm_delq_x (mutex, t->mutexes);
-         m->owner = unblock_from_queue (m->waiting);
-       }
-
       scm_i_pthread_mutex_unlock (&m->lock);
       ret = 1;
     }