Subject: bug#32257: 26.1; read-multiple-choice inf loops on mouse clicks

From: Andy Moreton <[email-masked]>
Date: Fri, 27 Jul 2018 12:35:27 +0100

On Fri 27 Jul 2018, Eli Zaretskii wrote:

>> From: Noam Postavsky <[email-masked]>
>> Date: Wed, 25 Jul 2018 21:25:46 -0400
>> Cc: lars ingebrigtsen <[email-masked]>, andy moreton <[email-masked]>
>> > Starting from emacs -Q, evaluate:
>> >
>> >     (require 'rmc)
>> >     (read-multiple-choice "choice? " '((?a "ay") (?b "bee") (?c "see")))
>> >
>> > then click the mouse somewhere instead of answering the prompt, notice
>> > Emacs maxing out CPU, hit C-g.  *Messages* will have a log of the prompt
>> > being repeated many times:
>> >
>> >     choice?  (ay, bee, [c] see, ?):  [4435 times]
>> 
>> The problem seems to be that read-char doesn't "use up" non-character
>> events, e.g., with the following:
>> 
>> (defun read-char-or-err ()
>>   (condition-case err
>>       (read-char)
>>     (error err)))
>> 
>> (list (read-char-or-err)
>>       (read-char-or-err)
>>       (read-char-or-err)
>>       (read-char-or-err)
>>       (read-char-or-err))
>> 
>> this returns ((error "Non-character input-event") (error "Non-character
>> input-event") ...) regardless of how many calls to read-char-or-err
>> there are.
>> 
>> The patch below fixes the inf looping, although it still doesn't allow
>> any other user interaction (unlike read-from-minibuffer).

I've tested this on emacs-26, and it fixes the original problem from NSM
that resulted in this bug report.

> If we want this change on emacs-26, we should carefully audit all the
> other users of rmc.el (and in generally, I'd prefer some more local
> change in nsm.el on the release branch).  We've had our share of
> subtle bugs introduced by switching to an "almost-compatible" method
> of reading input.

read-multiple choice only has two callers (`nsm-query-user' and
`message-fix-before-sending') in both master and emacs-26.

While I understand caution over changes to emacs-26, the only place that
needs fixing is read-multiple-choice. It seems odd to prefer changes
in its callers, that will more likely introduce additional bugs without
fixing the original issue.

    AndyM