bug#17577: 24.3.91; Error during redisplay prevent quitting.

[Stefan Monnier]

For:

  Error during redisplay: (#[128 "\300\301\"\210\300\302\"\207" [apply 
redisplay--update-region-highlights ignore nil] 4 nil nil] (#<window 174 on 
*helm locate*>)) signaled (quit)

the problem is probably that xdisp.c should set inhibit-quit while
running pre-redisplay-function.

But you also quote:

> Error during redisplay: (eval (helm-show-candidate-number [...]

This doesn't come from pre-redisplay-function but from updating the
mode-line, and I can't remember modifying this code.  And it seems that
this code has never run with inhibit-quit.  At least, that's what

(setq header-line-format '("" (:eval (format "inhibit-quit=%S" inhibit-quit))))

tells me in Emacs-23.4.  Actually, I see that inhibit-quit is also nil
while running jit-lock, which baffles me.  But indeed, if I add a slow
font-lock rule and hit C-g while it's processing that rule, I get:

   Error during redisplay: (quit)

So I wonder why this is new.  My best explanation is that it's actually
not new, but for some odd reason the timing has changed such that it
happens more often now.

> With this version of emacs (24.3.91) it is usable because I let-bounded
> `pre-redisplay-function' to nil,

Since pre-redisplay-function is new in 24.4, it's important we fix the
bugs it introduces, so please remove this workaround from helm.el.

>                     (:eval (helm-show-candidate-number
>                             (when (listp helm-mode-line-string)
>                               (car-safe helm-mode-line-string))))

The "when" check is redundant.

> but the message is still here, with Emacs-24.4.50, quitting is nearly
> impossible, and the message contain byte-code.

Sounds like the pre-redisplay-function message, then.  This one is
indeed new.

> Another strange thing is when hitting C-g on a long helm-grep process,
> C-g works normally but return "Emacs-lisp:" in the minibuffer when done.

Hmm... grepping around, "Emacs-lisp:" seems extremely unlikely, but
maybe you meant "Emacs-Lisp:" which I guess could potentially come from
the rarely used "menu in echo area" feature, i.e. "Emacs-Lisp" would
come from the name of a keymap.  Why/how this happens, I don't know.

I installed the patch below into the `emacs-24' branch.  Could you try
it and see if it solves the problem for you?


        Stefan


=== modified file 'src/ChangeLog'
--- src/ChangeLog       2014-05-22 05:00:39 +0000
+++ src/ChangeLog       2014-05-24 15:48:59 +0000
@@ -1,3 +1,11 @@
+2014-05-24  Stefan Monnier  <monnier@iro.umontreal.ca>
+
+       * xdisp.c: Bind inhibit-quit during pre-redisplay-function.
+       (safe__call, safe__call1, safe__eval): New functions.
+       (safe_call): Use it.
+       (prepare_menu_bars): Use it for pre-redisplay-function (bug#17577).
+       (display_mode_element): Same for `:eval'.
+
 2014-05-22  Paul Eggert  <eggert@cs.ucla.edu>
 
        Fix port to 32-bit AIX (Bug#17540).

=== modified file 'src/xdisp.c'
--- src/xdisp.c 2014-05-21 15:03:18 +0000
+++ src/xdisp.c 2014-05-24 15:45:51 +0000
@@ -2591,8 +2591,8 @@
    following.  Return the result, or nil if something went
    wrong.  Prevent redisplay during the evaluation.  */
 
-Lisp_Object
-safe_call (ptrdiff_t nargs, Lisp_Object func, ...)
+static Lisp_Object
+safe__call (bool inhibit_quit, ptrdiff_t nargs, Lisp_Object func, ...)
 {
   Lisp_Object val;
 
@@ -2615,6 +2615,8 @@
       GCPRO1 (args[0]);
       gcpro1.nvars = nargs;
       specbind (Qinhibit_redisplay, Qt);
+      if (inhibit_quit)
+       specbind (Qinhibit_quit, Qt);
       /* Use Qt to ensure debugger does not run,
         so there is no possibility of wanting to redisplay.  */
       val = internal_condition_case_n (Ffuncall, nargs, args, Qt,
@@ -2626,6 +2628,11 @@
   return val;
 }
 
+Lisp_Object
+safe_call (ptrdiff_t nargs, Lisp_Object func, ...)
+{
+  return safe__call (false, nargs, func);
+}
 
 /* Call function FN with one argument ARG.
    Return the result, or nil if something went wrong.  */
@@ -2633,7 +2640,13 @@
 Lisp_Object
 safe_call1 (Lisp_Object fn, Lisp_Object arg)
 {
-  return safe_call (2, fn, arg);
+  return safe__call (false, 2, fn, arg);
+}
+
+Lisp_Object
+safe__call1 (bool inhibit_quit, Lisp_Object fn, Lisp_Object arg)
+{
+  return safe__call (inhibit_quit, 2, fn, arg);
 }
 
 static Lisp_Object Qeval;
@@ -2641,7 +2654,13 @@
 Lisp_Object
 safe_eval (Lisp_Object sexpr)
 {
-  return safe_call1 (Qeval, sexpr);
+  return safe__call1 (false, Qeval, sexpr);
+}
+
+Lisp_Object
+safe__eval (bool inhibit_quit, Lisp_Object sexpr)
+{
+  return safe__call1 (inhibit_quit, Qeval, sexpr);
 }
 
 /* Call function FN with two arguments ARG1 and ARG2.
@@ -11549,7 +11568,7 @@
                }
            }
        }
-      safe_call1 (Vpre_redisplay_function, windows);
+      safe__call1 (true, Vpre_redisplay_function, windows);
     }
 
   /* Update all frame titles based on their buffer names, etc.  We do
@@ -21863,7 +21882,7 @@
            if (CONSP (XCDR (elt)))
              {
                Lisp_Object spec;
-               spec = safe_eval (XCAR (XCDR (elt)));
+               spec = safe__eval (true, XCAR (XCDR (elt)));
                n += display_mode_element (it, depth, field_width - n,
                                           precision - n, spec, props,
                                           risky);