Shane Mulligan
How to contact me: | |||||||
| |||||||
---------- Forwarded message ---------
From: Shane Mulligan <mullikine@gmail.com>
Date: Thu, May 20, 2021 at 1:05 AM
Subject: Re: bug#48500: 28.0.50; url-retrieve-synchronously exits abnormally due to pending keyboard input from terminal
To: Eli Zaretskii <eliz@gnu.org>
From: Shane Mulligan <mullikine@gmail.com>
Date: Thu, May 20, 2021 at 1:05 AM
Subject: Re: bug#48500: 28.0.50; url-retrieve-synchronously exits abnormally due to pending keyboard input from terminal
To: Eli Zaretskii <eliz@gnu.org>
Hey Eli,
I'm not sure where the quit is being generated but I will look into it.
Here are my insights.
** Original code
https://github.com/emacs-mirror/emacs/blob/HEAD/lisp/url/url.el
 292       ;; We used to use `sit-for' here, but in some cases it wouldn't
 293       ;; work because apparently pending keyboard input would always
 294       ;; interrupt it before it got a chance to handle process input.
 295       ;; `sleep-for' was tried but it lead to other forms of
 296       ;; hanging.  --Stef
 297       (unless (or (with-local-quit
 298              (accept-process-output proc 1))
 299             (null proc))
https://github.com/emacs-mirror/emacs/blob/HEAD/src/keyboard.c
 10395 DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
 10396     doc: /* Return t if command input is currently available with no wait.
 10397 Actually, the value is nil only if we can be sure that no input is available;
 10398 if there is a doubt, the value is t.
** I discovered that placing while-no-input here prevented quit from generating elsewhere
But then the overall function `url-retrieve-synchronously` would hang.
 112       (unless (or
 113            (while-no-input
 114             (with-local-quit
 115              (accept-process-output proc 1)))
 116            (null proc))
** Before discovering the 'fix' which is running keyboard-quit early (shown below), I avoided the hang by reading the key.
But reading and discarding the key wasn't a solution. I found that by doing the keyboard quit shown below instead of reading the key, the
keyboard input is preserved and somehow (unsure how), the pending input is pacified `accept-process-output` is 'safe' now to run.
  96       (with-local-quit
  97        (if (input-pending-p)
  98          (progn
  99           (setq counter (1+ counter))
 100           ;; (append-to-file (concat (char-to-string (read-key)) "\n"))
 101           (my-url-log (concat ">input pending" (str counter)))
 102           (if (> counter 20)
 103             (progn
 104              ;; (my-url-log (concat "QUIT" (str counter)))
 105              ;; (keyboard-quit))
 106            ;; This discards the input
 107            (read-key-sequence-vector nil nil t)
 108            (never
 109             (let ((k (read-key)))
 110              (my-url-log (concat "discarding: " (char-to-string k)))))
 111            ))))
 112       (unless (or
- 113 Â Â Â Â Â Â Â Â Â Â Â (while-no-input
 114             (with-local-quit
= 115 Â Â Â Â Â Â Â Â Â Â Â Â Â (accept-process-output proc 1)))
 116            (null procj))
** Clues
*** Back in 2006, it was advised in a different place to use input-pending-p instead of sit-for.
 6466 2006-09-12  Kim F. Storm  <storm@cua.dk>
 6467
 6468     * simple.el (next-error-highlight, next-error-highlight-no-select):
 6469     Fix spelling error.
 6470
 6471     * subr.el (sit-for): Rework to use input-pending-p and cond.
 6472     Return nil input is pending on entry also for SECONDS <= 0.
 6473     (while-no-input): Use input-pending-p instead of sit-for.
*** Quitting disabled when input-pendind-p is t
https://www.gnu.org/software/emacs/manual/html_node/elisp/Idle-Timers.html
https://github.com/emacs-mirror/emacs/blob/567c31121fdef6bdc8b645999a6ca1d994378c89/lisp/play/zone.el#L50
 49 ;; window. If the function loops, it *must* periodically check and
 50 ;; halt if `input-pending-p' is t (because quitting is disabled when
 51 ;; Emacs idle timers are run).
Shane Mulligan
I'm not sure where the quit is being generated but I will look into it.
Here are my insights.
** Original code
https://github.com/emacs-mirror/emacs/blob/HEAD/lisp/url/url.el
 292       ;; We used to use `sit-for' here, but in some cases it wouldn't
 293       ;; work because apparently pending keyboard input would always
 294       ;; interrupt it before it got a chance to handle process input.
 295       ;; `sleep-for' was tried but it lead to other forms of
 296       ;; hanging.  --Stef
 297       (unless (or (with-local-quit
 298              (accept-process-output proc 1))
 299             (null proc))
https://github.com/emacs-mirror/emacs/blob/HEAD/src/keyboard.c
 10395 DEFUN ("input-pending-p", Finput_pending_p, Sinput_pending_p, 0, 1, 0,
 10396     doc: /* Return t if command input is currently available with no wait.
 10397 Actually, the value is nil only if we can be sure that no input is available;
 10398 if there is a doubt, the value is t.
** I discovered that placing while-no-input here prevented quit from generating elsewhere
But then the overall function `url-retrieve-synchronously` would hang.
 112       (unless (or
 113            (while-no-input
 114             (with-local-quit
 115              (accept-process-output proc 1)))
 116            (null proc))
** Before discovering the 'fix' which is running keyboard-quit early (shown below), I avoided the hang by reading the key.
But reading and discarding the key wasn't a solution. I found that by doing the keyboard quit shown below instead of reading the key, the
keyboard input is preserved and somehow (unsure how), the pending input is pacified `accept-process-output` is 'safe' now to run.
  96       (with-local-quit
  97        (if (input-pending-p)
  98          (progn
  99           (setq counter (1+ counter))
 100           ;; (append-to-file (concat (char-to-string (read-key)) "\n"))
 101           (my-url-log (concat ">input pending" (str counter)))
 102           (if (> counter 20)
 103             (progn
 104              ;; (my-url-log (concat "QUIT" (str counter)))
 105              ;; (keyboard-quit))
 106            ;; This discards the input
 107            (read-key-sequence-vector nil nil t)
 108            (never
 109             (let ((k (read-key)))
 110              (my-url-log (concat "discarding: " (char-to-string k)))))
 111            ))))
 112       (unless (or
- 113 Â Â Â Â Â Â Â Â Â Â Â (while-no-input
 114             (with-local-quit
= 115 Â Â Â Â Â Â Â Â Â Â Â Â Â (accept-process-output proc 1)))
 116            (null procj))
** Clues
*** Back in 2006, it was advised in a different place to use input-pending-p instead of sit-for.
 6466 2006-09-12  Kim F. Storm  <storm@cua.dk>
 6467
 6468     * simple.el (next-error-highlight, next-error-highlight-no-select):
 6469     Fix spelling error.
 6470
 6471     * subr.el (sit-for): Rework to use input-pending-p and cond.
 6472     Return nil input is pending on entry also for SECONDS <= 0.
 6473     (while-no-input): Use input-pending-p instead of sit-for.
*** Quitting disabled when input-pendind-p is t
https://www.gnu.org/software/emacs/manual/html_node/elisp/Idle-Timers.html
https://github.com/emacs-mirror/emacs/blob/567c31121fdef6bdc8b645999a6ca1d994378c89/lisp/play/zone.el#L50
 49 ;; window. If the function loops, it *must* periodically check and
 50 ;; halt if `input-pending-p' is t (because quitting is disabled when
 51 ;; Emacs idle timers are run).
Shane Mulligan
How to contact me: | |||||||
| |||||||
On Wed, May 19, 2021 at 11:57 PM Eli Zaretskii <eliz@gnu.org> wrote:
> From: Shane Mulligan <mullikine@gmail.com>
> Date: Wed, 19 May 2021 18:48:09 +1200
>
> I may have resolved this issue with the following patch to `url-retrieve-synchronously`.
> What this achieves is to trigger a `quit` in a controlled environment rather than allowing it to occur when
> `accept-process-output` is run.
> It's not always wanted to trigger a quit when `(input-pending-p)` is `t`. But I noticed from placing
> `while-no-input` around `accept-process-output` to avoid the `quit` that `url-retrieve-synchronously` would
> then hang but with the controlled `quit` happening beforehand, `accept-process-output` no longer needs
> `while-no-input` around it. The end result is buttery smooth helm with no accidental `quit` from typing too
> fast. I think this may have resulted in GUI helm faster too.
Thanks, but what causes a quit in the first place?