bug#59055: closed ([PATCH] Fix possible deadlock.)

[GNU bug Tracking System]

Your message dated Sun, 20 Nov 2022 18:19:07 +0100
with message-id <87wn7p5yt0.fsf@gnu.org>
and subject line Re: bug#59055: [PATCH] Fix possible deadlock.
has caused the debbugs.gnu.org bug report #59055,
regarding [PATCH] Fix possible deadlock.
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
59055: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59055
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems
> --- Begin Message ---
>
>
>
> 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
>
>
>
>
> --- End Message ---
> --- Begin Message ---
>
>
>
> Subject: 
>
> Re: bug#59055: [PATCH] Fix possible deadlock.
>
>
>
>
> Date: 
>
> Sun, 20 Nov 2022 18:19:07 +0100
>
>
>
>
> User-agent: 
>
> Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
>
>
>
>
> Hi,
>
> Olivier Dion <olivier.dion@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’.
>
>
> --- End Message ---