Package: emacs;
Reported by: Daniel Colascione <dancol <at> dancol.org>
Date: Mon, 9 Jun 2025 20:50:02 UTC
Severity: normal
View this message in rfc822 format
From: Pip Cet <pipcet <at> protonmail.com> To: Daniel Colascione <dancol <at> dancol.org> Cc: 78737 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca> Subject: bug#78737: sit-for behavior changes when byte-compiled Date: Wed, 11 Jun 2025 10:32:00 +0000
"Daniel Colascione" <dancol <at> dancol.org> writes:
> On June 10, 2025 10:40:39 AM PDT, Pip Cet <pipcet <at> protonmail.com> wrote:
>>"Stefan Monnier" <monnier <at> iro.umontreal.ca> writes:
>>
>>>> BTW: the problem isn't just with transient. It also manifests with
>>>> read-extended-command! It's a nasty race that, AFAICT, has been with us
>>>> since the 90s. I think defining read_char to translate quits to quit_char
>>>> solves the problem.
>>>
>>> I like your way of thinking. I'm not completely sure it will solve
>>> world hunger, and it may come with regressions, but it's worth a try.
>>
>>I must be missing something: read_char translates quits to quit_char if
>>called with inhibit-quit bound to t, and never returns with quit-flag
>>set to t in that case.
>
> You shouldn't have to call it with inhibit-quit for it do that. It should just happen all the time.
But we don't usually want read-event to eat quits and report them by
returning quit_char. The old behavior gave me a choice.
(while t (read-event))
on your branch appears to hang the Emacs session unquittably (even
SIGUSR2 doesn't seem to work). It should permit quits, because that
code says nothing about wanting quits to be reported.
Reporting them as quit_char doesn't always make sense, since there are
other ways to generate a quit, such as SIGUSR2, and quit_char can
change.
> I don't think it should do that, it doesn't
>>match the quit-flag documentation, but it is what happens right now. Do
>>we really need a new function which is precisely equivalent to
>>
>>(let ((inhibit-quit t)) (read-event ...)) ?
>>
>>As for throw-on-input, I don't know what Daniel is proposing to do to
>>handle it. Is every caller of read-event supposed to check
>>throw-on-input and simulate it? How is that better than looking at
>>quit-flag, or simply keeping inhibit-quit bound for the critical
>>section?
>
> See the fix in the fix i mentioned a message ago. Now the read event
> functions detect that they're in a context in which quitting is
> inevitable and try to save the event before control even gets to
> Lisp. Should be transparent change.
It's pushing the event without the (cons t event) wrapper now? Won't
that change behavior of sit-for (and adding the wrapper would change
behavior of other users)?
Maybe we should just fix the original problem by making read_char call
maybe_quit instead of removing an event from the queue, if Vquit_flag is
set.
No reason to change anything about the API, read_char already calls Lisp
so quitting should not be a problem (famous last words).
Of course, the ability to peek at/wait for events would still be a good
thing, as would the ability to nest while-no-input invocations.
>>As for peeking at events, the easiest way seems to me to be to let-bind
>>unread-command-events to nil around a call to read-event. That'll make
>>it ignore them, read the next event, which you can then append to
>>unread-command-events or not depending on whether you want the command
>>loop to handle it.
>
> Isn't unread command events kind of lossy and not something we want to use all the time?
How is it "kind of lossy"? Anyway, you're using it on your branch, so
if it's unsafe it'll be unsafe for you, too.
If your concern is that unread-command-events might be updated
asynchronously as quit-flag is, I don't think we do that.
>>I agree it would make sense to separate inhibit-quit meaning "inhibit
>>acting on the quit flag" and inhibit-quit meaning "inhibit setting the
>>quit flag", but that seems a minor change.
>
> We have a lot of code that makes subtle assumptions about the meaning
> of the quit flag. I wouldn't change it lightly. The more local changes
> on my branch seem sufficient.
Er, you just did change it, in a way that breaks existing behavior. But
I agree, no changes such as that one are necessary.
Anyway, here are the minor changes to keyboard.c to avoid the original
problem (the third change is somewhat independent and avoids quitting in
kbd_buffer_get_event):
diff --git a/src/keyboard.c b/src/keyboard.c
index 5db11ad6379..5c65111f649 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3045,6 +3045,9 @@ read_char (int commandflag, Lisp_Object map,
timer_stop_idle ();
RESUME_POLLING;
+ input_was_pending = input_pending;
+ maybe_quit ();
+
if (NILP (c))
{
if (commandflag >= 0
@@ -4118,6 +4121,11 @@ kbd_buffer_get_event (KBOARD **kbp,
x_handle_pending_selection_requests ();
#endif
+ /* We're about to dequeue an event; if quit-flag is set, we might
+ never get around to handling it, so it would be lost. */
+ if (!NILP (Vquit_flag))
+ quit_throw_to_read_char (0);
+
if (CONSP (Vunread_command_events))
{
Lisp_Object first;
@@ -12992,7 +13000,7 @@ is_ignored_event (union buffered_input_event *event)
default: ignore_event = Qnil; break;
}
- return !NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events));
+ return !NILP (memq_no_quit (ignore_event, Vwhile_no_input_ignore_events));
}
static void syms_of_keyboard_for_pdumper (void);
Pip
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.