bug#19494: 25.0.50; infinite loop in readable_event on master branch

[Eli Zaretskii]

> Cc: eliz@gnu.org
> From: Noah Friedman <friedman@splode.com>
> Date: Fri, 02 Jan 2015 19:18:03 -0800 (PST)
> 
> That is, the entire kbd_buffer queue is BUFFER_SWITCH_EVENT events.
> Because of that, it means the sequence
> 
>           do {
>            ....
>               if (event == kbd_buffer + KBD_BUFFER_SIZE)
>                 event = kbd_buffer;
>           }
>         while (event != kbd_store_ptr);
> 
> in the function readable_event will, at least at times, loop forever.

It's strange no one reported anything similar since that changeset was
pushed.

> I don't have a trivial or always-reliable test case but I think the
> offending elisp is the following, running in a (non-idle) timer:
> 
>          (force-mode-line-update 'all)
>          (sit-for 0) ; force redisplay
> 
> I can change the elisp in question since I wrote it, but I'm not sure what
> the right way to do it is since it's not just running during idle times.

Why do you need that code, and in a timer on top of that?  It looks
strangely, and probably papers over some bug, either one that existed
in the past or maybe even an existing one.  If that bug still exists,
a bug report will be appreciated.

> In any case having emacs lock up in a non-recoverable way seems like a bug,
> since the only way out is to hit C-g and abort, or else go in with gdb
> on the running process and force a return.
> 
> Maybe this loop needs some kind of "didn't find any other event" flag to
> avoid looping, but I thought I'd check with the original committer first in
> case he has a better idea.

Stefan is in a better position to answer that, as my keyboard-event
foo is not strong enough.  You will see in the archives that I asked
him whether we should ignore these events before writing the patch.

One thing is clear: having input-pending-p return non-nil when there's
nothing in the queue but these buffer-switch events is a real bug that
causes quite a few problems.  So going back is not an option, I
think.

I do agree it would be nice to remove the possibility of looping, even
if it is a remote one.  And the logic you suggest is exactly what I
had in mind reading your description, so no, I don't have any better
idea.  Maybe Stefan does.