Recent keyboard changes breaks quitting

Recent keyboard changes breaks quitting

[Chong Yidong]

Hi Eli, somehow your latest changes to keyboard.c broke quitting:

M-x C-g
  => The cursor remains in the minibuffer.

A second C-g is needed to exit the minibuffer.
I verified that this behavior was introduced by

  revno: 110138
  fixes bugs: http://debbugs.gnu.org/12447 http://debbugs.gnu.org/12326
  committer: Eli Zaretskii <eliz@gnu.org>
  branch nick: trunk
  timestamp: Sat 2012-09-22 16:16:03 +0300
  message:
  Fix bugs #12447 and #12326 with infloop causes by idle timers, update docs.

  src/keyboard.c (timer_check_2): Move calculation of 'timers' and
  'idle_timers' from here ...
  (timer_check): ... to here.  Use Fcopy_sequence to copy the timer
  lists, to avoid infloops when the timer does something stupid,
  like reinvoke itself with the same or smaller time-out.

  lisp/emacs-lisp/timer.el (run-with-idle-timer)
  (timer-activate-when-idle): Warn against reinvoking an idle timer
  from within its own timer action.

  doc/lispref/os.texi (Idle Timers): Warn against reinvoking an idle timer
  from within its own timer action.

Re: Recent keyboard changes breaks quitting

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Sat, 22 Sep 2012 22:51:20 +0800
> 
> Hi Eli, somehow your latest changes to keyboard.c broke quitting:
> 
> M-x C-g
>   => The cursor remains in the minibuffer.
> 
> A second C-g is needed to exit the minibuffer.

Strange: I cannot reproduce this, neither on MS-Windows nor in a TTY
session on GNU/Linux.  Does the problem exist on a TTY on your
machine?

Re: Recent keyboard changes breaks quitting

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> M-x C-g
>>   => The cursor remains in the minibuffer.
>> 
>> A second C-g is needed to exit the minibuffer.
>
> Strange: I cannot reproduce this, neither on MS-Windows nor in a TTY
> session on GNU/Linux.  Does the problem exist on a TTY on your
> machine?

The problem indeed does not occur on a TTY.  I can reproduce it on both
a GTK+3 build and a no-X-toolkit build.

Re: Recent keyboard changes breaks quitting

[Jan Djärv]

2012-09-22 19:20, Chong Yidong skrev:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> M-x C-g
>>>    => The cursor remains in the minibuffer.
>>>
>>> A second C-g is needed to exit the minibuffer.
>>
>> Strange: I cannot reproduce this, neither on MS-Windows nor in a TTY
>> session on GNU/Linux.  Does the problem exist on a TTY on your
>> machine?
>
> The problem indeed does not occur on a TTY.  I can reproduce it on both
> a GTK+3 build and a no-X-toolkit build.
>

It seems to be some timing involved.  If I press C-g, Emacs says Quit in the 
minibuffer, but the cursor stays put.  If I press C-g after waiting a second 
or two, Emacs says Quit in the minibuffer, but the cursor still does not leave 
the minibuffer.  I can press as many C-g I want with the cursor in the 
minibuffer if I do it slowly (i.e. wait till the Quit text is replaced with M-x).
I have to press C-g twice close after each other to make the cursor leave the 
minibuffer (before Quit goes away).

	Jan D.

Re: Recent keyboard changes breaks quitting

[Chong Yidong]

Chong Yidong <cyd@gnu.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> M-x C-g
>>>   => The cursor remains in the minibuffer.
>>> 
>>> A second C-g is needed to exit the minibuffer.
>>
>> Strange: I cannot reproduce this, neither on MS-Windows nor in a TTY
>> session on GNU/Linux.  Does the problem exist on a TTY on your
>> machine?
>
> The problem indeed does not occur on a TTY.  I can reproduce it on both
> a GTK+3 build and a no-X-toolkit build.

Interestingly, the bug disappears if I remove Fcopy_sequence from

     idle_timers = Fcopy_sequence (Vtimer_idle_list);

in timer_check, and make no other changes, including keeping the
Fcopy_sequence for Vtimer_list two lines above.

Re: Recent keyboard changes breaks quitting

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Sun, 23 Sep 2012 01:35:31 +0800
> 
> Interestingly, the bug disappears if I remove Fcopy_sequence from
> 
>      idle_timers = Fcopy_sequence (Vtimer_idle_list);
> 
> in timer_check, and make no other changes, including keeping the
> Fcopy_sequence for Vtimer_list two lines above.

That's what I suspected.  (These calls to Fcopy_sequence is the only
real change in my commit, all the rest is just decorations and
consequences.)

A stab in the dark: does it help to BLOCK_INPUT around the call to
Fcopy_sequence?

If that doesn't help, I'd appreciate any pointers to where we switch
away from the minibuffer window following a C-g.

Re: Recent keyboard changes breaks quitting

[Eli Zaretskii]

> Date: Sat, 22 Sep 2012 21:56:15 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> A stab in the dark: does it help to BLOCK_INPUT around the call to
> Fcopy_sequence?

Also, does anything change if you disable blink-cursor-mode before
typing "M-x C-g"?

Re: Recent keyboard changes breaks quitting

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> A stab in the dark: does it help to BLOCK_INPUT around the call to
> Fcopy_sequence?

Nope.

> Also, does anything change if you disable blink-cursor-mode before
> typing "M-x C-g"?

Nope.  Turning off font-lock mode does, but I think this is only because
font-lock is is the only thing in my Emacs running an idle timer, in
this case jit-lock-context-fontify.  If I add any idle timer manually
(unrelated to font-lock), the same quitting problem appears.

Re: Recent keyboard changes breaks quitting

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Sun, 23 Sep 2012 10:53:37 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > A stab in the dark: does it help to BLOCK_INPUT around the call to
> > Fcopy_sequence?
> 
> Nope.
> 
> > Also, does anything change if you disable blink-cursor-mode before
> > typing "M-x C-g"?
> 
> Nope.  Turning off font-lock mode does, but I think this is only because
> font-lock is is the only thing in my Emacs running an idle timer, in
> this case jit-lock-context-fontify.  If I add any idle timer manually
> (unrelated to font-lock), the same quitting problem appears.

Hmm... so what idle timer runs when blink-cursor-mode is disabled?
Can you set a breakpoint in timer_check and see which timers end up in
the list?  (The breakpoint should pp the list and continue, to not
disrupt the operation.)

Re: Recent keyboard changes breaks quitting

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> Nope.  Turning off font-lock mode does, but I think this is only because
>> font-lock is is the only thing in my Emacs running an idle timer, in
>> this case jit-lock-context-fontify.  If I add any idle timer manually
>> (unrelated to font-lock), the same quitting problem appears.
>
> Hmm... so what idle timer runs when blink-cursor-mode is disabled?

I just told you: jit-lock-context-fontify.

But it's not a misbehaving idle timer, if that's what you're wondering
about.  It is possible to reproduce the bug when *any* idle timer is
around, e.g. putting this in the init file

  (global-font-lock-mode 0)
  (defun foo-timer-fun (&rest ignored)
    (message "%d" foo))

and running

  M-: (run-with-idle-timer 2 nil #'foo-timer-fun) RET
  M-x C-g

My attempts at tracing indicate that your timer_check change causes
significant changes in wait_reading_process_output's order of execution
when there is an idle timer present.  I think the problem occurs in the
call in process.c:110145, with

wait_reading_process_output
  -> detect_input_pending_run_timers
    -> get_input_pending
      -> readable_events
        -> timer_check

This timer_check does not call process_pending_signals.  Now, the same
call to timer_check does call process_pending_signals, apparently via
the QUIT macro.  Not sure if this helps.

Re: Recent keyboard changes breaks quitting

[Chong Yidong]

I found the problem.  The Fcopy_sequence in timer_check can QUIT,
jumping back to the command loop.  If this happens when timer_check is
from inside wait_reading_process_output, it screws up the
wait_reading_process_output's handling of the quit flag.

=== modified file 'src/keyboard.c'
*** src/keyboard.c	2012-09-22 13:16:03 +0000
--- src/keyboard.c	2012-09-23 08:22:06 +0000
***************
*** 4496,4501 ****
--- 4496,4504 ----
    Lisp_Object timers, idle_timers;
    struct gcpro gcpro1, gcpro2;
  
+   Lisp_Object tem = Vinhibit_quit;
+   Vinhibit_quit = Qt;
+ 
    /* We use copies of the timers' lists to allow a timer to add itself
       again, without locking up Emacs if the newly added timer is
       already ripe when added.  */
***************
*** 4508,4513 ****
--- 4511,4518 ----
    else
      idle_timers = Qnil;
  
+   Vinhibit_quit = tem;
+ 
    GCPRO2 (timers, idle_timers);
  
    do

Re: Recent keyboard changes breaks quitting

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Sun, 23 Sep 2012 16:33:33 +0800
> 
> I found the problem.  The Fcopy_sequence in timer_check can QUIT,
> jumping back to the command loop.  If this happens when timer_check is
> from inside wait_reading_process_output, it screws up the
> wait_reading_process_output's handling of the quit flag.

Great, thanks!

Re: Recent keyboard changes breaks quitting

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Date: Sun, 23 Sep 2012 01:35:31 +0800
> Cc: emacs-devel@gnu.org
> 
> Interestingly, the bug disappears if I remove Fcopy_sequence from
> 
>      idle_timers = Fcopy_sequence (Vtimer_idle_list);

Is it possible that the call to timer-event-handler, which modifies
the timer (via cancel-timer-internal) and the timer list, interacts
badly with the copied timer list, because Fcopy_alist doesn't copy the
list elements?  If so, perhaps making a deep copy of the list will fix
the problem?