GNU bug report logs - #64819
30.0.50; condition-wait not interruptible

Previous Next

To reply to this bug, email your comments to 64819 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; condition-wait not interruptible
Date: Mon, 24 Jul 2023 08:32:04 +0200

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

When Emacs is blocked in condition-wait, then C-g doesn't seem to work.
E.g. Emacs hangs and pressing C-g has no apparent effect when executing
the attached file with:

  emacs -Q -l deadlock.el -f deadlock

Mabye sys_cond_wait could call pthread_cond_timedwait with a fairly long
timeout, say one second, and repeatedly check if C-g was pressed.

[deadlock.el (application/emacs-lisp, inline)]

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

Configured using:
 'configure --with-xpm=ifavailable --with-jpeg=ifavailable
 --with-gif=ifavailable --with-tiff=ifavailable'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LIBSELINUX
LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG SECCOMP SOUND
SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 GTK3
ZLIB

Important settings:
  value of $LANG: C.UTF-8
  locale-coding-system: utf-8-unix

Message #8 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Mon, 24 Jul 2023 15:10:24 +0300

> From: Helmut Eller <eller.helmut <at> gmail.com>
> Date: Mon, 24 Jul 2023 08:32:04 +0200
> 
> When Emacs is blocked in condition-wait, then C-g doesn't seem to work.
> E.g. Emacs hangs and pressing C-g has no apparent effect when executing
> the attached file with:
> 
>   emacs -Q -l deadlock.el -f deadlock
> 
> Mabye sys_cond_wait could call pthread_cond_timedwait with a fairly long
> timeout, say one second, and repeatedly check if C-g was pressed.

How will we know what to do with C-g if some thread runs Lisp, while
one or more other threads are stuck in condition-wait? wouldn't you
expect in this case to have C-g go to the running thread?

Message #11 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Mon, 24 Jul 2023 14:58:37 +0200

On Mon, Jul 24 2023, Eli Zaretskii wrote:

> How will we know what to do with C-g if some thread runs Lisp, while
> one or more other threads are stuck in condition-wait? wouldn't you
> expect in this case to have C-g go to the running thread?

We could say that C-g sets quit-flag and causes all blocking calls to
condition-wait to return nil (spurious wakeup).  At that point all
threads are conceptually running.  Then the first thread (unspecified
which one) who calls maybe_quit() finishes handling C-g and clears
quit-flag afterwards.  Those threads who don't feel prepared to handle
C-g can bind inhibit-quit.

Helmut

Message #14 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Mon, 24 Jul 2023 16:34:17 +0300

> From: Helmut Eller <eller.helmut <at> gmail.com>
> Cc: 64819 <at> debbugs.gnu.org
> Date: Mon, 24 Jul 2023 14:58:37 +0200
> 
> On Mon, Jul 24 2023, Eli Zaretskii wrote:
> 
> > How will we know what to do with C-g if some thread runs Lisp, while
> > one or more other threads are stuck in condition-wait? wouldn't you
> > expect in this case to have C-g go to the running thread?
> 
> We could say that C-g sets quit-flag and causes all blocking calls to
> condition-wait to return nil (spurious wakeup).  At that point all
> threads are conceptually running.  Then the first thread (unspecified
> which one) who calls maybe_quit() finishes handling C-g and clears
> quit-flag afterwards.  Those threads who don't feel prepared to handle
> C-g can bind inhibit-quit.

I don't think we can allow more than one thread at a time to run the
parts of the Lisp interpreter that lead to maybe_quit.

Also, I don't think what you describe, even if it were possible, is
what users expect: they expect that the thread which is running is
interrupted, and either exits or handles the quit, and all the other
threads still wait for the condition var.

So I think to do anything smarter in the deadlock situation you
describe we'd need to detect the deadlock first.  Once we do that
(which isn't easy: perhaps do that in the signal handler?), we'd need
to decide which of the deadlocked threads to free, which is also not
trivial.  Hmmm...

Btw, did you try your recipe in a Unix TTY?  There, C-g actually
delivers SIGINT to Emacs, so you might see a different behavior (or a
crash ;-).

Message #17 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Mon, 24 Jul 2023 16:57:05 +0200

On Mon, Jul 24 2023, Eli Zaretskii wrote:

>> From: Helmut Eller <eller.helmut <at> gmail.com>
>> We could say that C-g sets quit-flag and causes all blocking calls to
>> condition-wait to return nil (spurious wakeup).  At that point all
>> threads are conceptually running.  Then the first thread (unspecified
>> which one) who calls maybe_quit() finishes handling C-g and clears
>> quit-flag afterwards.  Those threads who don't feel prepared to handle
>> C-g can bind inhibit-quit.
>
> I don't think we can allow more than one thread at a time to run the
> parts of the Lisp interpreter that lead to maybe_quit.

I didn't suggest that.  Nor did I suggest that the thread scheduler
should switch away from the currently running thread.

What I did suggest is that the thread blocked in condition-wait is
considered runnable.  So that the thread scheduler is allowed to pick
this thread the next time when somebody calls thread-yield or
condition-wait.

To the thread it will look like a spurious wakeup (i.e. condition-wait
returned but the condition isn't actually true) but Lisp code must
already be prepared for such a situation.

The bytecode interpreter calls maybe_quit before every call or backward
branch, so maybe_quit will be called very soon after the spurious
wakeup.

> Also, I don't think what you describe, even if it were possible, is
> what users expect: they expect that the thread which is running is
> interrupted, and either exits or handles the quit, and all the other
> threads still wait for the condition var.

Maybe we can agree on this: when only one thread exists and it is
blocked in condition-wait, then condition-wait should be interruptible
by C-g.

For the situation where some threads are blocked in condition-wait and
one other thread is running, I think that running thread would call
maybe_quit and clear quite-flag before calling thread-yield.  The other
threads would observe spurious wakeups as soon as they are allowed to
run.

> So I think to do anything smarter in the deadlock situation you
> describe we'd need to detect the deadlock first.  Once we do that
> (which isn't easy: perhaps do that in the signal handler?), we'd need
> to decide which of the deadlocked threads to free, which is also not
> trivial.  Hmmm...

I doubt that deadlock detection is possible in the general case.
E.g. how could we possibly know that a timer is or isn't going to call
condition-notify in 5 seconds?

> Btw, did you try your recipe in a Unix TTY?  There, C-g actually
> delivers SIGINT to Emacs, so you might see a different behavior (or a
> crash ;-).

When I run the recipe with: "emacs -nw -l deadlock.el -f deadlock" then
I see the emergency escape feature kick in.  Only after the second C-g
(of course).  A single C-g doesn't seem to do anything.

Helmut

Message #20 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Mon, 24 Jul 2023 19:23:31 +0300

> From: Helmut Eller <eller.helmut <at> gmail.com>
> Cc: 64819 <at> debbugs.gnu.org
> Date: Mon, 24 Jul 2023 16:57:05 +0200
> 
> On Mon, Jul 24 2023, Eli Zaretskii wrote:
> 
> >> From: Helmut Eller <eller.helmut <at> gmail.com>
> >> We could say that C-g sets quit-flag and causes all blocking calls to
> >> condition-wait to return nil (spurious wakeup).  At that point all
> >> threads are conceptually running.  Then the first thread (unspecified
> >> which one) who calls maybe_quit() finishes handling C-g and clears
> >> quit-flag afterwards.  Those threads who don't feel prepared to handle
> >> C-g can bind inhibit-quit.
> >
> > I don't think we can allow more than one thread at a time to run the
> > parts of the Lisp interpreter that lead to maybe_quit.
> 
> I didn't suggest that.  Nor did I suggest that the thread scheduler
> should switch away from the currently running thread.

You did say "At that point all threads are conceptually running."
Perhaps I misunderstood what you meant.

> What I did suggest is that the thread blocked in condition-wait is
> considered runnable.  So that the thread scheduler is allowed to pick
> this thread the next time when somebody calls thread-yield or
> condition-wait.

We don't have a scheduler.  The threads "schedule themselves", in the
sense that the first thread that succeeds to grab the global lock gets
to run, and the others wait for the lock to be released.

So for this to work, the C-g handler will have to release some thread.

> Maybe we can agree on this: when only one thread exists and it is
> blocked in condition-wait, then condition-wait should be interruptible
> by C-g.

Yes, this is simpler.

> For the situation where some threads are blocked in condition-wait and
> one other thread is running, I think that running thread would call
> maybe_quit and clear quite-flag before calling thread-yield.  The other
> threads would observe spurious wakeups as soon as they are allowed to
> run.

I'm not sure I follow here.  First, no one guarantees that a running
thread will call thread-yield; it could call sit-for or somesuch
instead.  And second if C-g only sets quit-flag, then it will not be
able to release a stuck thread; and if it does release a stuck thread,
it will somehow need to stop the running one, or we will have more
than one thread running.

> > So I think to do anything smarter in the deadlock situation you
> > describe we'd need to detect the deadlock first.  Once we do that
> > (which isn't easy: perhaps do that in the signal handler?), we'd need
> > to decide which of the deadlocked threads to free, which is also not
> > trivial.  Hmmm...
> 
> I doubt that deadlock detection is possible in the general case.
> E.g. how could we possibly know that a timer is or isn't going to call
> condition-notify in 5 seconds?

We are talking about a situation where the user typed C-g, so we have
some indication that the situation is not normal.

> > Btw, did you try your recipe in a Unix TTY?  There, C-g actually
> > delivers SIGINT to Emacs, so you might see a different behavior (or a
> > crash ;-).
> 
> When I run the recipe with: "emacs -nw -l deadlock.el -f deadlock" then
> I see the emergency escape feature kick in.  Only after the second C-g
> (of course).  A single C-g doesn't seem to do anything.

As expected, thanks.

Message #23 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Tue, 25 Jul 2023 10:06:40 +0200

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

On Mon, Jul 24 2023, Eli Zaretskii wrote:

[...]
> So for this to work, the C-g handler will have to release some thread.

Here is a patch that works good enough for me:

[0001-Make-condition-wait-interruptible-by-C-g.patch (text/x-diff, inline)]

From 4de3198c10c4efaeaffdf43ba5e5b0f1729a7f09 Mon Sep 17 00:00:00 2001
From: Helmut Eller <eller.helmut <at> gmail.com>
Date: Tue, 25 Jul 2023 10:03:53 +0200
Subject: [PATCH] Make condition-wait interruptible by C-g

Code like

  (let* ((mutex (make-mutex))
         (cvar (make-condition-variable mutex)))
    (with-mutex mutex
      (condition-wait cvar)))

will block in pthread_cond_wait.  The problem is that
pthread_cond_wait may or may not return when it gets interrupted
by a signal (SIGIO).  On Linux it doesn't return and so even if a
signal handler sets pending_signals=true nobody processes those
pending signals.

The patch modifies the signal handler so that it will force a
spurious wakeup when the current thread is blocked in
condition-wait.

* src/keyboard.c (handle_input_available_signal)
(handle_interrupt): Call maybe_awake_current_thread.
* src/thread.c (maybe_awake_current_thread): New.
* src/thread.h (maybe_awake_current_thread): New prototype.
---
 src/keyboard.c |  8 +++++++-
 src/thread.c   | 16 ++++++++++++++++
 src/thread.h   |  1 +
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index 41cda2e65de..f45bafa96c0 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -7745,6 +7745,10 @@ handle_input_available_signal (int sig)
 
   if (input_available_clear_time)
     *input_available_clear_time = make_timespec (0, 0);
+
+#ifdef THREADS_ENABLED
+  maybe_awake_current_thread ();
+#endif
 }
 
 static void
@@ -11556,8 +11560,10 @@ handle_interrupt (bool in_signal_handler)
   /* If we were called from a signal handler, we must be in the main
      thread, see deliver_process_signal.  So we must make sure the
      main thread holds the global lock.  */
-  if (in_signal_handler)
+  if (in_signal_handler) {
     maybe_reacquire_global_lock ();
+    maybe_awake_current_thread();
+  }
 #endif
   if (waiting_for_input && !echoing)
     quit_throw_to_read_char (in_signal_handler);
diff --git a/src/thread.c b/src/thread.c
index b8ca56fd372..0bd949f5779 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -172,6 +172,22 @@ maybe_reacquire_global_lock (void)
     }
 }
 
+/* This is called from keyboard.c when it sets pending_signals=true.
+   If the current thread is waiting, we create a spurious wakeup by
+   broadcasting on wait_condvar.  This is necessary because
+   pthread_cond_wait may or may not return if it was interrupted by a
+   signal (SIGIO).  Without the wakeup, nobody would process a
+   potential C-g.
+*/
+void
+maybe_awake_current_thread (void)
+{
+  if (current_thread->wait_condvar != NULL)
+    {
+      sys_cond_broadcast (current_thread->wait_condvar);
+    }
+}
+
 
 
 static void
diff --git a/src/thread.h b/src/thread.h
index 9b14cc44f35..60f601a6248 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -311,6 +311,7 @@ extern void finalize_one_thread (struct thread_state *state);
 extern void finalize_one_mutex (struct Lisp_Mutex *);
 extern void finalize_one_condvar (struct Lisp_CondVar *);
 extern void maybe_reacquire_global_lock (void);
+extern void maybe_awake_current_thread (void);
 
 extern void init_threads (void);
 extern void syms_of_threads (void);
-- 
2.39.2

Message #26 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Tue, 25 Jul 2023 15:18:29 +0300

> From: Helmut Eller <eller.helmut <at> gmail.com>
> Cc: 64819 <at> debbugs.gnu.org
> Date: Tue, 25 Jul 2023 10:06:40 +0200
> 
> On Mon, Jul 24 2023, Eli Zaretskii wrote:
> 
> [...]
> > So for this to work, the C-g handler will have to release some thread.
> 
> Here is a patch that works good enough for me:

Thanks.  If you are currently working on or using some packages that
use Lisp threads, please run with this patch for a while and come back
in a couple of weeks if you see no problems with it; then we can
install this on master.  (It will need a small addition for
MS-Windows, but that can be handled later.)  A test for this, if
possible, would also be appreciated, even if it can only be run
manually (we have the test/manual/ directory for those).

If you do not intend to use threads any time soon, I guess we can
install this on master and let someone else be the guinea pig...

A couple of minor stylistic comments:

> -  if (in_signal_handler)
> +  if (in_signal_handler) {
>      maybe_reacquire_global_lock ();
> +    maybe_awake_current_thread();
> +  }

Please use the GNU style of braces:

  if (something)
    {
      do_1;
      do_2;
    }

> +void
> +maybe_awake_current_thread (void)
> +{
> +  if (current_thread->wait_condvar != NULL)
> +    {
> +      sys_cond_broadcast (current_thread->wait_condvar);
> +    }

We don't use braces when the body of 'if' has just one statement.

Message #29 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Tue, 25 Jul 2023 14:59:38 +0200

On Tue, Jul 25 2023, Eli Zaretskii wrote:

> Thanks.  If you are currently working on or using some packages that
> use Lisp threads, please run with this patch for a while and come back
> in a couple of weeks if you see no problems with it; then we can
> install this on master.  (It will need a small addition for
> MS-Windows, but that can be handled later.)  A test for this, if
> possible, would also be appreciated, even if it can only be run
> manually (we have the test/manual/ directory for those).

I will try to write a simple multi-threaded HTTP client or proxy to
exercise the threading primitives a bit.

> If you do not intend to use threads any time soon, I guess we can
> install this on master and let someone else be the guinea pig...

The patch doesn't handle the situation where current_thread==NULL.  Not
sure how that can happen, but the patch is definitely not ready for
master.

Helmut

Message #32 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Sat, 2 Sep 2023 14:58:27 -0700

Helmut Eller <eller.helmut <at> gmail.com> writes:

> On Tue, Jul 25 2023, Eli Zaretskii wrote:
>
>> Thanks.  If you are currently working on or using some packages that
>> use Lisp threads, please run with this patch for a while and come back
>> in a couple of weeks if you see no problems with it; then we can
>> install this on master.  (It will need a small addition for
>> MS-Windows, but that can be handled later.)  A test for this, if
>> possible, would also be appreciated, even if it can only be run
>> manually (we have the test/manual/ directory for those).
>
> I will try to write a simple multi-threaded HTTP client or proxy to
> exercise the threading primitives a bit.
>
>> If you do not intend to use threads any time soon, I guess we can
>> install this on master and let someone else be the guinea pig...
>
> The patch doesn't handle the situation where current_thread==NULL.  Not
> sure how that can happen, but the patch is definitely not ready for
> master.

Did you get any further with this?

Message #35 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Helmut Eller <eller.helmut <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Sun, 03 Sep 2023 21:53:21 +0200

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

On Sat, Sep 02 2023, Stefan Kangas wrote:

> Did you get any further with this?

Well, a bit.  I'm currently using the attached patch and for testing the
attached shell script.  The patch is good enough to pass these tests.  I
don't know if it breaks something else.

I had to change maybe_reacquire_global_lock to make the tests pass.  The
idea behind maybe_reacquire_global_lock seems dubious: waiting for a
lock in a signal handler seems, well, problematic.

Anyway, the patch is work in progress and only useful to me.

(The patch also includes changes for Windows, but I only tested those
manually and under Wine.  For the Windows console version, C-g doesn't
interrupt condition-wait.  Ordinary endless loops don't seem be
interruptible there either.  So, maybe this is acceptable.)

Helmut

[interruptible-condition-wait.patch (text/x-diff, attachment)]

[cnd-wait-test.sh (text/x-sh, attachment)]

[wait.el (text/plain, attachment)]

Message #38 received at 64819 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64819 <at> debbugs.gnu.org
Subject: Re: bug#64819: 30.0.50; condition-wait not interruptible
Date: Wed, 6 Sep 2023 02:35:03 -0700

Helmut Eller <eller.helmut <at> gmail.com> writes:

> Anyway, the patch is work in progress and only useful to me.

Thanks, it's good to see that you're still working on this.  Please do
let us know when you have something that you feel is closer to being
ready for install.

I'll leave commenting on your patch to other people, as I'm not familiar
with this code.

Previous Next

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