bug#48500: Fwd: bug#48500: 28.0.50; url-retrieve-synchronously exits abnormally due to pending keyboard input from terminal

[Shane Mulligan <mullikine@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5513 bytes --]

Shane Mulligan

How to contact me:
🇦🇺 00 61 421 641 250
🇳🇿 00 64 21 1462 759 <+64-21-1462-759>
mullikine@gmail.com


---------- 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>


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

How to contact me:
🇦🇺 00 61 421 641 250
🇳🇿 00 64 21 1462 759 <+64-21-1462-759>
mullikine@gmail.com


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?
>

[-- Attachment #2: Type: text/html, Size: 10326 bytes --]