GNU bug report logs - #59055
[PATCH] Fix possible deadlock.

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Olivier Dion <olivier.dion <at> polymtl.ca>
Subject: bug#59055: closed (Re: bug#59055: [PATCH] Fix possible deadlock.)
Date: Sun, 20 Nov 2022 17:20:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#59055: [PATCH] Fix possible deadlock.

which was filed against the guile package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 59055 <at> debbugs.gnu.org.

-- 
59055: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59055
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Ludovic Courtès <ludo <at> gnu.org>
To: Olivier Dion <olivier.dion <at> polymtl.ca>
Cc: 59055-done <at> debbugs.gnu.org
Subject: Re: bug#59055: [PATCH] Fix possible deadlock.
Date: Sun, 20 Nov 2022 18:19:07 +0100

Hi,

Olivier Dion <olivier.dion <at> polymtl.ca> skribis:

> If we got interrupted while waiting on our condition variable, we unlock
> the kernel mutex momentarily while executing asynchronous operations
> before putting us back into the waiting queue.
>
> However, we have to retry acquiring the mutex before getting back into
> the queue, otherwise it's possible that we wait indefinitely since
> nobody could be the owner for a while.
>
> * libguile/threads.c (lock_mutex): Try acquring the mutex after signal
> interruption.

Looks reasonable to me; applied.

Did you try to come up with a reproducer?  That would be awesome but I
guess it’s hard because you need to trigger EINTR at the right point.

Thanks,
Ludo’.

[Message part 3 (message/rfc822, inline)]

From: Olivier Dion <olivier.dion <at> polymtl.ca>
To: bug-guile <at> gnu.org
Cc: Olivier Dion <olivier.dion <at> polymtl.ca>
Subject: [PATCH] Fix possible deadlock.
Date: Sat,  5 Nov 2022 12:59:23 -0400

If we got interrupted while waiting on our condition variable, we unlock
the kernel mutex momentarily while executing asynchronous operations
before putting us back into the waiting queue.

However, we have to retry acquiring the mutex before getting back into
the queue, otherwise it's possible that we wait indefinitely since
nobody could be the owner for a while.

* libguile/threads.c (lock_mutex): Try acquring the mutex after signal
interruption.
---
 libguile/threads.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/libguile/threads.c b/libguile/threads.c
index 280d306bf..0f5cf2ed5 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -1022,14 +1022,7 @@ lock_mutex (enum scm_mutex_kind kind, struct scm_mutex *m,
 
         if (err == 0)
           {
-            if (scm_is_eq (m->owner, SCM_BOOL_F))
-              {
-                m->owner = current_thread->handle;
-                scm_i_pthread_mutex_unlock (&m->lock);
-                return SCM_BOOL_T;
-              }
-            else
-              continue;
+            goto maybe_acquire;
           }
         else if (err == ETIMEDOUT)
           {
@@ -1041,7 +1034,7 @@ lock_mutex (enum scm_mutex_kind kind, struct scm_mutex *m,
             scm_i_pthread_mutex_unlock (&m->lock);
             scm_async_tick ();
             scm_i_scm_pthread_mutex_lock (&m->lock);
-            continue;
+            goto maybe_acquire;
           }
         else
           {
@@ -1050,6 +1043,14 @@ lock_mutex (enum scm_mutex_kind kind, struct scm_mutex *m,
             errno = err;
             SCM_SYSERROR;
           }
+
+      maybe_acquire:
+        if (scm_is_eq (m->owner, SCM_BOOL_F))
+          {
+            m->owner = current_thread->handle;
+            scm_i_pthread_mutex_unlock (&m->lock);
+            return SCM_BOOL_T;
+          }
       }
 }
 #undef FUNC_NAME
-- 
2.38.0

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.