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

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

[Noah Friedman]

Ever since commit 614beeecf29d16c08f66a4f82b6085be90df8a74 (c. Nov 01 2014)
I keep finding "idle" emacs processes running in a tight loop in
keyboard.c:readable_event after I come back to my session the following
morning.  In those cases, kbd_buffer is full of the same event, in my case:

        $17 = {
          kind = BUFFER_SWITCH_EVENT,
          part = scroll_bar_nowhere,
          code = 0,
          modifiers = 0,
          x = 0,
          y = 0,
          timestamp = 0,
          frame_or_window = 12962290,
          arg = 12962290
        }

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.

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

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.

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

[Noah Friedman]

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

It's very old code, circa 1995.  Since it predates the v21 redisplay engine
rewrite, I have no idea if it's still necessary; I'm testing that now.

>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 concur.  I don't want to revert it, it may just need some refinement.

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

[Lars Ingebrigtsen]

Noah Friedman <friedman@splode.com> writes:

>>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.
>
> It's very old code, circa 1995.  Since it predates the v21 redisplay engine
> rewrite, I have no idea if it's still necessary; I'm testing that now.

This was six years ago -- are you still seeing this issue in more recent
Emacs versions?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Noah Friedman]

I'll have to go dig around and get back to you; I've forgotten the context and what I might have done about it.

*From: *Lars Ingebrigtsen <larsi@gnus.org>
*To: *Noah Friedman <friedman@splode.com>
*CC: *eliz@gnu.org; 19494@debbugs.gnu.org
*Date: *Jul 13, 2021 11:06:36
*Subject: *Re: bug#19494: 25.0.50; infinite loop in readable_event on master branch

> Noah Friedman <friedman@splode.com> writes:
> 
>>> 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.
>> 
>> It's very old code, circa 1995.  Since it predates the v21 redisplay engine
>> rewrite, I have no idea if it's still necessary; I'm testing that now.
> 
> This was six years ago -- are you still seeing this issue in more recent
> Emacs versions?
> 

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

[Lars Ingebrigtsen]

Noah Friedman <noah@splode.com> writes:

> I'll have to go dig around and get back to you; I've forgotten the
> context and what I might have done about it.

This was a month ago -- any progress?  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Noah Friedman]

I don't remember running into that particular behavior in the past year,
but in any case I see that quite a bit of the input event code has changed
since I reported this.  So if I ever come across it again I would file a
new bug.  Please close this one.

Thanks.

In <87y28x7ep8.fsf@gnus.org> 2021-08-19 15:21:23+0200, Lars Ingebrigtsen <larsi@gnus.org> writes:
>Noah Friedman <noah@splode.com> writes:
>
>> I'll have to go dig around and get back to you; I've forgotten the
>> context and what I might have done about it.
>
>This was a month ago -- any progress?  :-)

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

[Lars Ingebrigtsen]

Noah Friedman <friedman@splode.com> writes:

> I don't remember running into that particular behavior in the past year,
> but in any case I see that quite a bit of the input event code has changed
> since I reported this.  So if I ever come across it again I would file a
> new bug.  Please close this one.

OK; done.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no