* bug#64819: 30.0.50; condition-wait not interruptible @ 2023-07-24 6:32 Helmut Eller 2023-07-24 12:10 ` Eli Zaretskii 0 siblings, 1 reply; 12+ messages in thread From: Helmut Eller @ 2023-07-24 6:32 UTC (permalink / raw) To: 64819 [-- Attachment #1: Type: text/plain, Size: 348 bytes --] 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. [-- Attachment #2: deadlock.el --] [-- Type: application/emacs-lisp, Size: 170 bytes --] [-- Attachment #3: Type: text/plain, Size: 450 bytes --] 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-24 6:32 bug#64819: 30.0.50; condition-wait not interruptible Helmut Eller @ 2023-07-24 12:10 ` Eli Zaretskii 2023-07-24 12:58 ` Helmut Eller 0 siblings, 1 reply; 12+ messages in thread From: Eli Zaretskii @ 2023-07-24 12:10 UTC (permalink / raw) To: Helmut Eller; +Cc: 64819 > From: Helmut Eller <eller.helmut@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? ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-24 12:10 ` Eli Zaretskii @ 2023-07-24 12:58 ` Helmut Eller 2023-07-24 13:34 ` Eli Zaretskii 0 siblings, 1 reply; 12+ messages in thread From: Helmut Eller @ 2023-07-24 12:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 64819 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-24 12:58 ` Helmut Eller @ 2023-07-24 13:34 ` Eli Zaretskii 2023-07-24 14:57 ` Helmut Eller 0 siblings, 1 reply; 12+ messages in thread From: Eli Zaretskii @ 2023-07-24 13:34 UTC (permalink / raw) To: Helmut Eller; +Cc: 64819 > From: Helmut Eller <eller.helmut@gmail.com> > Cc: 64819@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 ;-). ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-24 13:34 ` Eli Zaretskii @ 2023-07-24 14:57 ` Helmut Eller 2023-07-24 16:23 ` Eli Zaretskii 0 siblings, 1 reply; 12+ messages in thread From: Helmut Eller @ 2023-07-24 14:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 64819 On Mon, Jul 24 2023, Eli Zaretskii wrote: >> From: Helmut Eller <eller.helmut@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-24 14:57 ` Helmut Eller @ 2023-07-24 16:23 ` Eli Zaretskii 2023-07-25 8:06 ` Helmut Eller 0 siblings, 1 reply; 12+ messages in thread From: Eli Zaretskii @ 2023-07-24 16:23 UTC (permalink / raw) To: Helmut Eller; +Cc: 64819 > From: Helmut Eller <eller.helmut@gmail.com> > Cc: 64819@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@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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-24 16:23 ` Eli Zaretskii @ 2023-07-25 8:06 ` Helmut Eller 2023-07-25 12:18 ` Eli Zaretskii 0 siblings, 1 reply; 12+ messages in thread From: Helmut Eller @ 2023-07-25 8:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 64819 [-- Attachment #1: Type: text/plain, Size: 172 bytes --] 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: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Make-condition-wait-interruptible-by-C-g.patch --] [-- Type: text/x-diff, Size: 3180 bytes --] From 4de3198c10c4efaeaffdf43ba5e5b0f1729a7f09 Mon Sep 17 00:00:00 2001 From: Helmut Eller <eller.helmut@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); + } +} + \f 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-25 8:06 ` Helmut Eller @ 2023-07-25 12:18 ` Eli Zaretskii 2023-07-25 12:59 ` Helmut Eller 0 siblings, 1 reply; 12+ messages in thread From: Eli Zaretskii @ 2023-07-25 12:18 UTC (permalink / raw) To: Helmut Eller; +Cc: 64819 > From: Helmut Eller <eller.helmut@gmail.com> > Cc: 64819@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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-25 12:18 ` Eli Zaretskii @ 2023-07-25 12:59 ` Helmut Eller 2023-09-02 21:58 ` Stefan Kangas 0 siblings, 1 reply; 12+ messages in thread From: Helmut Eller @ 2023-07-25 12:59 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 64819 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-07-25 12:59 ` Helmut Eller @ 2023-09-02 21:58 ` Stefan Kangas 2023-09-03 19:53 ` Helmut Eller 0 siblings, 1 reply; 12+ messages in thread From: Stefan Kangas @ 2023-09-02 21:58 UTC (permalink / raw) To: Helmut Eller, Eli Zaretskii; +Cc: 64819 Helmut Eller <eller.helmut@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? ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-09-02 21:58 ` Stefan Kangas @ 2023-09-03 19:53 ` Helmut Eller 2023-09-06 9:35 ` Stefan Kangas 0 siblings, 1 reply; 12+ messages in thread From: Helmut Eller @ 2023-09-03 19:53 UTC (permalink / raw) To: Stefan Kangas; +Cc: Eli Zaretskii, 64819 [-- Attachment #1: Type: text/plain, Size: 798 bytes --] 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: interruptible-condition-wait.patch --] [-- Type: text/x-diff, Size: 6004 bytes --] diff --git a/src/keyboard.c b/src/keyboard.c index 6ab1cdebc12..d8262c779c2 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -8099,6 +8099,11 @@ handle_input_available_signal (int sig) if (input_available_clear_time) *input_available_clear_time = make_timespec (0, 0); + +#ifdef THREADS_ENABLED + maybe_reacquire_global_lock (); + maybe_awake_current_thread (); +#endif } static void @@ -12076,7 +12081,10 @@ handle_interrupt (bool in_signal_handler) thread, see deliver_process_signal. So we must make sure the main thread holds the global lock. */ if (in_signal_handler) - maybe_reacquire_global_lock (); + { + 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/process.c b/src/process.c index 08cb810ec13..a25071a482a 100644 --- a/src/process.c +++ b/src/process.c @@ -5473,7 +5473,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, update_status (wait_proc); if (wait_proc && ! EQ (wait_proc->status, Qrun) - && ! connecting_status (wait_proc->status)) + && ! connecting_status (wait_proc->status) + && ! EQ (wait_proc->status, Qlisten)) { bool read_some_bytes = false; diff --git a/src/systhread.c b/src/systhread.c index caa4dfd4443..fa652abbb30 100644 --- a/src/systhread.c +++ b/src/systhread.c @@ -273,6 +273,9 @@ sys_thread_yield (void) #include <mbctype.h> #include "w32term.h" +/* From w32xfns.c */ +extern HANDLE interrupt_handle; + /* Cannot include <process.h> because of the local header by the same name, sigh. */ uintptr_t _beginthread (void (__cdecl *)(void *), unsigned, void *); @@ -310,7 +313,9 @@ sys_cond_init (sys_cond_t *cond) cond->events[CONDV_SIGNAL] = CreateEvent (NULL, FALSE, FALSE, NULL); /* Manual-reset event for broadcast. */ cond->events[CONDV_BROADCAST] = CreateEvent (NULL, TRUE, FALSE, NULL); - if (!cond->events[CONDV_SIGNAL] || !cond->events[CONDV_BROADCAST]) + cond->events[CONDV_INTERRUPT] = interrupt_handle; + if (!cond->events[CONDV_SIGNAL] || !cond->events[CONDV_BROADCAST] + || !cond->events[CONDV_INTERRUPT]) return; InitializeCriticalSection ((LPCRITICAL_SECTION)&cond->wait_count_lock); cond->initialized = true; @@ -333,15 +338,15 @@ sys_cond_wait (sys_cond_t *cond, sys_mutex_t *mutex) /* Release the mutex and wait for either the signal or the broadcast event. */ LeaveCriticalSection ((LPCRITICAL_SECTION)mutex); - wait_result = WaitForMultipleObjects (2, cond->events, FALSE, INFINITE); + wait_result + = WaitForMultipleObjects (CONDV_MAX, cond->events, FALSE, INFINITE); /* Decrement the wait count and see if we are the last thread waiting on the condition variable. */ EnterCriticalSection ((LPCRITICAL_SECTION)&cond->wait_count_lock); cond->wait_count--; - last_thread_waiting = - wait_result == WAIT_OBJECT_0 + CONDV_BROADCAST - && cond->wait_count == 0; + last_thread_waiting = wait_result == WAIT_OBJECT_0 + CONDV_BROADCAST + && cond->wait_count == 0; LeaveCriticalSection ((LPCRITICAL_SECTION)&cond->wait_count_lock); /* Broadcast uses a manual-reset event, so when the last thread is diff --git a/src/systhread.h b/src/systhread.h index b8f078df071..7e9ac83f951 100644 --- a/src/systhread.h +++ b/src/systhread.h @@ -55,7 +55,13 @@ typedef struct { unsigned long SpinCount; } w32thread_critsect; -enum { CONDV_SIGNAL = 0, CONDV_BROADCAST = 1, CONDV_MAX = 2 }; +enum +{ + CONDV_SIGNAL = 0, + CONDV_BROADCAST = 1, + CONDV_INTERRUPT = 2, + CONDV_MAX = 3 +}; typedef struct { /* Count of threads that are waiting for this condition variable. */ diff --git a/src/thread.c b/src/thread.c index b8ca56fd372..efe21702b5f 100644 --- a/src/thread.c +++ b/src/thread.c @@ -158,21 +158,46 @@ acquire_global_lock (struct thread_state *self) void maybe_reacquire_global_lock (void) { + eassert (sys_thread_equal (sys_thread_self (), main_thread.s.thread_id)); + /* SIGINT handler is always run on the main thread, see deliver_process_signal, so reflect that in our thread-tracking variables. */ - current_thread = &main_thread.s; + struct thread_state *self = &main_thread.s; - if (current_thread->not_holding_lock) + if (self->not_holding_lock) { - struct thread_state *self = current_thread; - acquire_global_lock (self); - current_thread->not_holding_lock = 0; + self->not_holding_lock = 0; + eassert (current_thread == self); } + + if (current_thread == NULL) + post_acquire_global_lock (self); + + eassert (!self->not_holding_lock); + eassert (current_thread != NULL); } -\f +/* 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) +{ + eassert (sys_thread_equal (sys_thread_self (), main_thread.s.thread_id)); + eassert (!main_thread.s.not_holding_lock); + eassert (current_thread != NULL); + + struct thread_state *t = current_thread; + + if (t->wait_condvar != NULL) + sys_cond_broadcast (t->wait_condvar); +} static void lisp_mutex_init (lisp_mutex_t *mutex) 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); [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: cnd-wait-test.sh --] [-- Type: text/x-sh, Size: 1627 bytes --] #!/bin/bash # Test whether condition-wait is interruptible by C-g. # # This script requires two tools: xdotool and dotool. # # xdotool: is included in Debian # # dotool: I installed it manually from https://git.sr.ht/~geb/dotool. # As dotool requires permissions to access /dev/input, I use sudo to # run it. # # The script output follows TAP syntax (https://testanything.org/) and # could be parsed for example with prove(1). set -o errexit # Ask sudo password now and remember it for later sudo dotool --version >/dev/null TMPFILE=$(mktemp) TESTNR=0 function check { local PROG="$1" FUN="$2" READYMSG="$3" $PROG -Q -l wait.el --eval "($FUN \"$TMPFILE\")" & local EMACSPID=$! trap "rm -f $TMPFILE; kill $EMACSPID 2>/dev/null || :" EXIT # Wait until Emacs is ready until [ "$(cat $TMPFILE)" == "$READYMSG" ] ; do sleep 0.2 done # Set focus on Emacs xdotool search --sync --onlyvisible --all \ --pid $EMACSPID --name '' windowfocus # Send C-g to input device echo 'key ctrl+g' | sudo dotool wait $EMACSPID DESC="$TESTNR - $PROG $FUN" case "$(cat $TMPFILE)" in QUIT | QUIT1 | QUIT2) echo ok $DESC;; *) cat $TMPFILE >2 echo not ok $DESC ;; esac } declare -a PROGS PROGS=(emacs 'xterm -e emacs -nw') declare -A READYMSGS READYMSGS=( [wait-forever]=WAITING [spawn-child-then-wait]='WAITING1 TERMINATING2' [two-threads-waiting]='WAITING1 WAITING2' ) echo 1..$((${#PROGS[@]} * ${#READYMSGS[@]})) for prog in "${PROGS[@]}" ; do for fun in "${!READYMSGS[@]}" ; do TESTNR=$(($TESTNR + 1)) check "$prog" $fun "${READYMSGS[$fun]}" done done [-- Attachment #4: wait.el --] [-- Type: text/plain, Size: 1483 bytes --] ;; -*- lexical-binding:t -*- (defun wait-forever (filename) (unwind-protect (condition-case e (let* ((mutex (make-mutex)) (cvar (make-condition-variable mutex))) (with-mutex mutex (write-region "WAITING" nil filename nil 0) (while t (condition-wait cvar)))) (quit (write-region "QUIT" nil filename nil 0))) (kill-emacs))) (defun spawn-child-then-wait (filename) (make-thread (lambda () (write-region " TERMINATING2" nil filename t 0))) (unwind-protect (condition-case e (let* ((mutex (make-mutex)) (cvar (make-condition-variable mutex))) (with-mutex mutex (write-region "WAITING1" nil filename nil 0) (while t (condition-wait cvar)))) (quit (write-region "QUIT1" nil filename nil 0))) (kill-emacs))) (defun two-threads-waiting (filename) (make-thread (lambda () (unwind-protect (condition-case e (let* ((mutex (make-mutex)) (cvar (make-condition-variable mutex))) (with-mutex mutex (write-region " WAITING2" nil filename t 0) (while t (condition-wait cvar)))) (quit (write-region "QUIT2" nil filename nil 0))) (kill-emacs)))) (unwind-protect (condition-case e (let* ((mutex (make-mutex)) (cvar (make-condition-variable mutex))) (with-mutex mutex (write-region "WAITING1" nil filename nil 0) (while t (condition-wait cvar)))) (quit (write-region " QUIT1" nil filename t 0))) (kill-emacs))) ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#64819: 30.0.50; condition-wait not interruptible 2023-09-03 19:53 ` Helmut Eller @ 2023-09-06 9:35 ` Stefan Kangas 0 siblings, 0 replies; 12+ messages in thread From: Stefan Kangas @ 2023-09-06 9:35 UTC (permalink / raw) To: Helmut Eller; +Cc: Eli Zaretskii, 64819 Helmut Eller <eller.helmut@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. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-09-06 9:35 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-24 6:32 bug#64819: 30.0.50; condition-wait not interruptible Helmut Eller 2023-07-24 12:10 ` Eli Zaretskii 2023-07-24 12:58 ` Helmut Eller 2023-07-24 13:34 ` Eli Zaretskii 2023-07-24 14:57 ` Helmut Eller 2023-07-24 16:23 ` Eli Zaretskii 2023-07-25 8:06 ` Helmut Eller 2023-07-25 12:18 ` Eli Zaretskii 2023-07-25 12:59 ` Helmut Eller 2023-09-02 21:58 ` Stefan Kangas 2023-09-03 19:53 ` Helmut Eller 2023-09-06 9:35 ` Stefan Kangas
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.