Signal `quit' in a `font-lock-fontify-region-function'

Signal `quit' in a `font-lock-fontify-region-function'

[Paul Pogonyshev]

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

Hi,

I'm having the following, unfortunately unreproducible problem: inside a
custom font-locking function condition `quit' _sometimes_ cannot be
triggered with C-g. I'm sure it has nothing to do with `inhibit-quit'. For
the same function in one Emacs instance it is triggerable, in another ---
not. If in a certain instance it "breaks" (i.e. C-g no longer triggers
`quit'), it seems to stay broken forever.

This is notoriously difficult to investigate. I came up with the following
pseudo-mode (usage: M-x pretend-fontification-mode RET in a large buffer):

(define-derived-mode pretend-fontification-mode nil "PRETEND-FONTIFICATION"
  "..."
  (setq font-lock-defaults '(nil))
  (set (make-local-variable 'font-lock-fontify-region-function)
       (lambda (from to _verbose)
         ;; (message ...) doesn't seem to do anything
         (with-current-buffer "*Messages*" (with-silent-modifications
(insert (format "\nPRETEND-FONTIFYING %s-%s\n" from to))))
         (let (x)
           (dotimes (k 10000000)  ; long delay in pure Lisp, i.e. not
`sleep-for'
             (setq x k))))))

but as I said it is not reproducible: sometimes mashing C-g results in the
following in *Messages* buffer:

Error during redisplay: (jit-lock-function ##) signaled (quit)

sometimes it doesn't. I also tried `(condition-case ... (quit ...))' inside
font-locking function, but result is the same: sometimes the handler is
triggered, sometimes it is not and I cannot work out any pattern. I also
tried printing out values that could be relevant (inhibit-quit, quit-flag,
(input-pending-p), unread-command-events), but they appear to be the same
regardless of whether `quit' is still triggerable or not.

Does anyone has any idea what's going on?

Paul

P.S.: I'm not subscribed to the list, please CC answers to me.

[-- Attachment #2: Type: text/html, Size: 2330 bytes --]

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Stefan Monnier]

> Error during redisplay: (jit-lock-function ##) signaled (quit)

This is indeed the behavior I expect (tho I don't really like it, but
I never dared to change jit-lock to set inhibit-quit).

> sometimes it doesn't.

Is there a difference in behavior in this respect when you try it in the
GUI vs in a tty?  How 'bout in another OS?

Does it happen more often if you wait a while until you starting banging
on C-g?  E.g. it might be that the event buffer is full (see patch below)?


        Stefan


diff --git a/src/keyboard.c b/src/keyboard.c
index 5f2b7afe6d..405790312d 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3568,12 +3568,15 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
         {
           /* Don't read keyboard input until we have processed kbd_buffer.
              This happens when pasting text longer than KBD_BUFFER_SIZE/2.  */
+          fprintf (stderr, "Too many pending events, putting keyboard on hold!\n");
           hold_keyboard_input ();
           unrequest_sigio ();
           stop_polling ();
         }
 #endif	/* subprocesses */
     }
+  else
+    fprintf (stderr, "event buffer full: dropping event!\n");
 
   Lisp_Object ignore_event;
 

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Paul Pogonyshev]

Found this answer to my question in the list archives. I'm not
subscribed, please CC to me.

> > Error during redisplay: (jit-lock-function ##) signaled (quit)
>
> This is indeed the behavior I expect (tho I don't really like it, but
I never dared to change jit-lock to set inhibit-quit).
>
> > sometimes it doesn't.

Is there a difference in behavior in this respect when you try it in the
GUI vs in a tty?  How 'bout in another OS?

After recompiling with your patch I cannot reproduce it. Maybe it was
fixed by some other change, but more likely that I made a mistake and
the problem was never there. I could have been confused by my own
`message' replacement since it didn't insert at the buffer end, but
rather at the point.

Anyway, why I needed this. It is not impossible that during
development you write a fontification function that is broken and e.g.
falls into infinite loop. You can sort of abort it with C-g, but then
fontification code marks the whole region as unfontified (since the
function never finished), calls the function again, it gets stuck...
And the only way you can end this is by killing Emacs process. Or at
least I don't know any better way.

It is also possible to accidentally stumble into a finite, but
extremely long fontification. For example, if you open a
megabyte-large minified JavaScript file. I believe also one of the
many Python modes was susceptible at some point.

I was trying to write some workaround for my own mode, because it
targets log files, which can be hundreds of megabytes in size.
Additionally, the mode puts filtering into fontification process,
meaning that in fontification callback it can hide everything (if the
filter is too strict). And then the callback gets called again and
again, chewing through all the megabytes with no apparent way to stop
it until it finally finishes minutes later.

But would it make sense to adjust fontification code to disable
fontification in the current buffer if fontification function is
killed with C-g several times? I.e. instead of some custom workaround
for my mode only, add something generic to Emacs core. This would give
the user a chance to kill offending buffer rather than the whole
Emacs.

Paul

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Stefan Monnier]

> But would it make sense to adjust fontification code to disable
> fontification in the current buffer if fontification function is
> killed with C-g several times?

Yes but: there's the obvious question of "how many C-g are
necessary", as well as making sure this doesn't misfire just because the
user hits C-g several times for some other reason.

IOW, to do this, we'd need to make sure that Emacs was busy font-locking
this one buffer during all this time, or something like that.

Also, I get the impression that we'd want this kind of behavior for more
than just font-lock.


        Stefan

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Paul Pogonyshev]

> Yes but: there's the obvious question of "how many C-g are
> necessary", as well as making sure this doesn't misfire just because the
> user hits C-g several times for some other reason.
>
> IOW, to do this, we'd need to make sure that Emacs was busy font-locking
> this one buffer during all this time, or something like that.

I certainly didn't mean one C-g. More like three presses within one
second and all during font-lock. Still could result in false positives, but
I believe an occasional unwanted broken fontlocking is better than
outright hung Emacs.

Could also be customizable as everything else.

I think a larger (apparent) problem is to let user know what's going on.
Naively issued messages would get overridden with "Quit" in the echo
area, because the natural response (at least mine) in such situation is
to mash C-g continuosly, not do it accurately and watch if there is any
change. And those "excessive" C-g would result in "Quit" message.
This is also part of the reason I haven't continued with implementing
this just for my mode, but rather feel it would be better as a general
Emacs feature.

> Also, I get the impression that we'd want this kind of behavior for more
> than just font-lock.

Yeah, I have encountered similar problems with timers due to some recent
bug. I mean, the bug has been fixed already, but I still had to restart Emacs
a couple of times, and what guarantees there will be no similar bugs in the
future?

Paul

On Tue, 28 May 2019 at 19:50, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > But would it make sense to adjust fontification code to disable
> > fontification in the current buffer if fontification function is
> > killed with C-g several times?
>
> Yes but: there's the obvious question of "how many C-g are
> necessary", as well as making sure this doesn't misfire just because the
> user hits C-g several times for some other reason.
>
> IOW, to do this, we'd need to make sure that Emacs was busy font-locking
> this one buffer during all this time, or something like that.
>
> Also, I get the impression that we'd want this kind of behavior for more
> than just font-lock.
>
>
>         Stefan
>

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Stefan Monnier]

> I think a larger (apparent) problem is to let user know what's going on.
> Naively issued messages would get overridden with "Quit" in the echo
> area, because the natural response (at least mine) in such situation is
> to mash C-g continuosly, not do it accurately and watch if there is any
> change. And those "excessive" C-g would result in "Quit" message.
> This is also part of the reason I haven't continued with implementing
> this just for my mode, but rather feel it would be better as a general
> Emacs feature.

Right, here's how I could imagine a "desirable" behavior:

- everytime some operation may block Emacs, it should "register"
  somewhere (probably some let-binding of a global var is all it takes).
- After hitting C-g, a timer is started.
- If this timer is reached before the C-g has had to chance to be
  processed, emit a message in the echo area explaining what Emacs is
  currently doing (based on the "registered" information above).
- when we reach the Nth C-g in a row within the same "registered"
  operation, we abort the operation (e.g. by calling an ad-hoc
  function provided while "registering").


-- Stefan

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Paul Pogonyshev]

Found this answer to my question in the list archives. I'm not
subscribed, please CC to me.

> Right, here's how I could imagine a "desirable" behavior:
>
> - everytime some operation may block Emacs, it should "register"
>   somewhere (probably some let-binding of a global var is all it takes).
> - After hitting C-g, a timer is started.
> - If this timer is reached before the C-g has had to chance to be
>   processed, emit a message in the echo area explaining what Emacs is
>   currently doing (based on the "registered" information above).
> - when we reach the Nth C-g in a row within the same "registered"
>   operation, we abort the operation (e.g. by calling an ad-hoc
>   function provided while "registering").

I'm not sure if this is flexible enough.  I don't like the idea of C-g
already invoking some code (that e.g. disables font-locking).  Please
read on to see a usecase that I feel would be impossible to support
with your proposal.

My fontification function often extends the fontification region
considerably and then returns `(jit-lock-bounds START . END)', as
required in the documentation.  In my case this makes fontification
much faster, because the function itself is very fast and most of the
time is spent in various setups (both in fontlocking code and the
function itself), not in traversing through the region to be
fontified.

So, I'd like to be able to _handle_ `quit' signal in the function
itself and stop extending the region and return immediately, so that I
don't throw away work on a considerable region of text that has been
done already.  However, fontlocking code shouldn't discard the results
(as otherwise this is pointless).  So, I cannot resignal `quit', I
have to return normally.  But I also don't want to "eat" this signal
and hide C-g press from Emacs' monitoring code.  So basically, I want
font-locking still continue to work for a while, even if C-g has been
pressed too many times, but if so, abort soon.

I'm not sure if it is the best solution, but I'd propose C-g to be
counted not when `quit' signal is caught, but when it would be
emitted, even if `inhibit-quit' is t.  And font-locking code would
look something like this:

    (let ((count-C-g-presses t))  ; Instead of "registering" as you
proposed, I think we just
                                  ; need to tell C-g handler to count
presses while inside
                                  ; certain blocks of code.
      (condition-case error
          (let ((fontified-region (funcall actual-handler ...)))
            ;; Process the fontified region
            ...)
        (error ;; This logs any errors inside the handler, including uncaught
               ;; `quit' signals.  Just as now.
               ...))
      (when (too-many-C-g-recently)  ; This would count both C-g that
resulted in (signal 'quit ...)
                                     ; and those that resulted in
`(setq quit-flag t)'.
        ;; The below forms are roughly what would be "registered" in your
        ;; proposal.  I feel leaving it up to the caller gives more
        ;; flexibility in actually permorming "abort everything" tasks.
        (setq font-lock-mode nil)
        (message "Too many C-g, font-locking disabled")))

I think I could try to implement this myself, but on the other hand
I'm certainly not familiar with Emacs internals.  So I would gladly
give up this task to someone else who would like to do it.

Paul

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Stefan Monnier]

> I'm not sure if this is flexible enough.  I don't like the idea of C-g
> already invoking some code (that e.g. disables font-locking).

In my suggestion, C-g wouldn't invoke any significant code.  All it
would do is setup a timer, and when the timer expires a message would be
emitted in the echo area to inform the user what it is that Emacs is
currently doing.
It definitely wouldn't disable font-locking right away.

The disabling would only be in response to a "large" number of C-g,
which presumably should only be needed in extreme circumstances
(basically when currently the user ends up killing Emacs instead).

> So, I'd like to be able to _handle_ `quit' signal in the function
> itself and stop extending the region and return immediately, so that I
> don't throw away work on a considerable region of text that has been
> done already.

I don't understand why you say "don't throw away work on a considerable
region of text that has been done already": if you're in the middle of
extending the region, presumably no "work" has been done yet.

> However, fontlocking code shouldn't discard the results
> (as otherwise this is pointless).  So, I cannot resignal `quit', I
> have to return normally.  But I also don't want to "eat" this signal
> and hide C-g press from Emacs' monitoring code.  So basically, I want
> font-locking still continue to work for a while, even if C-g has been
> pressed too many times, but if so, abort soon.

Could `while-no-input` be usable in this case?

> I'm not sure if it is the best solution, but I'd propose C-g to be
> counted not when `quit' signal is caught, but when it would be
> emitted, even if `inhibit-quit' is t.  And font-locking code would
> look something like this:
>
>     (let ((count-C-g-presses t))  ; Instead of "registering" as you
> proposed, I think we just
>                                   ; need to tell C-g handler to count
> presses while inside
>                                   ; certain blocks of code.
>       (condition-case error
>           (let ((fontified-region (funcall actual-handler ...)))
>             ;; Process the fontified region
>             ...)
>         (error ;; This logs any errors inside the handler, including uncaught
>                ;; `quit' signals.  Just as now.
>                ...))
>       (when (too-many-C-g-recently)  ; This would count both C-g that
> resulted in (signal 'quit ...)
>                                      ; and those that resulted in
> `(setq quit-flag t)'.

I think if C-g resulted in (signal 'quit ...), then presumably we exited
this code, so the only really interesting case should be the one where
we just have `quit-flag` set to t.

Maybe we should change the C-g code so it sets `quit-flag` to the
number of C-g that are pending.

>         ;; The below forms are roughly what would be "registered" in your
>         ;; proposal.  I feel leaving it up to the caller gives more
>         ;; flexibility in actually permorming "abort everything" tasks.
>         (setq font-lock-mode nil)
>         (message "Too many C-g, font-locking disabled")))

Maybe when "registering", the code could provide a function to
execute instead of "aborting".


        Stefan

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Paul Pogonyshev]

On Sat, 29 Jun 2019 at 23:26, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > I'm not sure if this is flexible enough.  I don't like the idea of C-g
> > already invoking some code (that e.g. disables font-locking).
>
> [...] It definitely wouldn't disable font-locking right away.
>
> The disabling would only be in response to a "large" number of C-g,
> which presumably should only be needed in extreme circumstances
> (basically when currently the user ends up killing Emacs instead).

So, basically, as I understand, C-g handler in your proposal would
look like (I know it is in C code, but...), simplified:

    (defun handle-C-g (...)
      (when registered-many-C-g-callback  ; variable set by e.g.
font-locking code
        (setq num-C-g-presses (1+ num-C-g-presses))
        (when (> num-C-g-presses 3)
          (funcall registered-many-C-g-callback))))

and font-locking code would be roughly

    (let ((registered-many-C-g-callback (lambda () (setq font-lock-mode nil))))
      ; call fontification function, process its result etc.
      )

That's what I meant: here `handle-C-g' already invokes some callback
that higher level (in this case font-locking) registers.

I propose that instead font-locking code simply says "I will process
multiple C-g presses somehow, please count them" by let-binding
`count-C-g-presses' to t.  The idea is to not invoke the "abort"
callback immediately when the 3rd C-g is pressed, but instead let the
iteration finish gracefully and _then_ let the higher-level code
(font-locking) abort or not based on all the information it has by
now.  Usually this information would be just result of
`(too-many-C-g-recently)', but could be anything in the future, so we
are more flexible.

> > So, I'd like to be able to _handle_ `quit' signal in the function
> > itself and stop extending the region and return immediately, so that I
> > don't throw away work on a considerable region of text that has been
> > done already.
>
> I don't understand why you say "don't throw away work on a considerable
> region of text that has been done already": if you're in the middle of
> extending the region, presumably no "work" has been done yet.

For example, my function decides to fontify 1000 lines of text and by
the time C-g is pressed (triggering the signal) it has already
fontified 500 of them (this is already more than what font-locking
code wanted, because I extended the requested region).  I'd like to be
able to notify font-locking code about the work that has been done,
even if I planned to do even more.  If I raise `quit' signal or let
C-g raise it by default, all this work is thrown away.

> Could `while-no-input` be usable in this case?

This macro uses `with-local-quit', which, as I understand will rethrow
signal `quit' after the body.  As a result, font-locking code will
still see my function terminate non-locally, deem the whole invocation
failed and set property `fontified' to nil.  So no, to achive what I
want I need to let my function return normally somehow.

>
> > I'm not sure if it is the best solution, but I'd propose C-g to be
> > counted not when `quit' signal is caught, but when it would be
> > emitted, even if `inhibit-quit' is t.  And font-locking code would
> > look something like this: [...]
>
> I think if C-g resulted in (signal 'quit ...), then presumably we exited
> this code, so the only really interesting case should be the one where
> we just have `quit-flag` set to t.

Yes, but the point is in that I want to avoid C-g resulting in the
signal by binding `inhibit-quit' inside my fontification function.
But still have C-g handler note that C-g is pressed and let
fontification code find if it is being pressed too often.

> Maybe we should change the C-g code so it sets `quit-flag` to the
> number of C-g that are pending.

This generally sounds like a good idea: I'm always in favour of having
more information.  However, in my case it is not enough.

I guess it is easier to explain by pseudocode of my fontification
function:

    (defun my--fontify-region (region-start region-end _loudly)
      ;; Very fast, so always work on large regions to avoid setup overhead.
      (setq region-end (+ region-start 50000))
      ;; Here comes some internal setup.
      ...
      (let ((fontified-up-to region-start)
            (inhibit-quit    t))
        ;; While `quit' is inhibitted, the below loop will still be
aborted on C-g.
        (while (and (< fontified-up-to region-end) (null quit-flag))
          ...  ; do some work on the next small chunk of text; this is
very fast,
               ; but as the region is quite big, the loop is iterated many times
          )
        ;; Always exit normally.  Font-locking code will find out if
we suppressed
        ;; a C-g through other means, i.e. using (too-many-C-g-recently).
        (setq quit-flag nil)
        `(jit-lock-bounds ,region-start . ,fontified-up-to)))

Note that the function doesn't suppress any other signals/errors.

Paul

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Stefan Monnier]

> So, basically, as I understand, C-g handler in your proposal would
> look like (I know it is in C code, but...), simplified:
>
>     (defun handle-C-g (...)
>       (when registered-many-C-g-callback  ; variable set by e.g.
> font-locking code
>         (setq num-C-g-presses (1+ num-C-g-presses))
>         (when (> num-C-g-presses 3)
>           (funcall registered-many-C-g-callback))))

Not quite, more like:

    (defvar registered-many-C-g-callback (lambda () (signal 'quit nil)))
    (defvar registered-many-C-g nil)

    (defun handle-C-g (...)
      (when registered-many-C-g
        (setq num-C-g-presses (1+ num-C-g-presses))
        (when (> num-C-g-presses 3)
          (funcall registered-many-C-g-callback))))

> and font-locking code would be roughly
>
>     (let ((registered-many-C-g-callback (lambda () (setq font-lock-mode nil))))
>       ; call fontification function, process its result etc.
>       )

More like:

    (let ((registered-many-C-g (cons "font-locking..." registered-many-C-g)))
      ; call fontification function, process its result etc.
      )

But, yes, maybe it would provide a callback to disable font-lock-mode,
or just catch the `quit` signal and disable font-lock-mode.

> `count-C-g-presses' to t.  The idea is to not invoke the "abort"
> callback immediately when the 3rd C-g is pressed, but instead let the
> iteration finish gracefully and _then_ let the higher-level code

Your bounds-extension code could re-bind registered-many-C-g-callback to
do something less drastic than disable font-lock-mode (or exit brutally
from font-lock).


        Stefan

Re: Signal `quit' in a `font-lock-fontify-region-function'

[Paul Pogonyshev]

> Your bounds-extension code could re-bind registered-many-C-g-callback to
> do something less drastic than disable font-lock-mode (or exit brutally
> from font-lock).

The point is that I _do_ want to disable font-lock-mode after N C-g presses.
But I also want my fontification function return normally after C-g (including
Nth press) to preserve non-trivial amount of work that has been done.

In principle, as long as font-locking code can finish fontification iteration
normally even if it is disabled in the middle of it (because of Nth C-g), then
your proposed solution is also fine for my usecase. I.e.:

    (let ((registered-many-C-g (cons "font-locking..." registered-many-C-g)))
      ; call fontification function, process its result etc.
      ; nowhere inside this let-block it checks if `font-lock-mode'
      ; is still enabled. only next iteration may be cancelled by
      ; setting `font-lock-mode' to nil, not the ongoing one.
      )

On Sun, 30 Jun 2019 at 02:18, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > So, basically, as I understand, C-g handler in your proposal would
> > look like (I know it is in C code, but...), simplified:
> >
> >     (defun handle-C-g (...)
> >       (when registered-many-C-g-callback  ; variable set by e.g.
> > font-locking code
> >         (setq num-C-g-presses (1+ num-C-g-presses))
> >         (when (> num-C-g-presses 3)
> >           (funcall registered-many-C-g-callback))))
>
> Not quite, more like:
>
>     (defvar registered-many-C-g-callback (lambda () (signal 'quit nil)))
>     (defvar registered-many-C-g nil)
>
>     (defun handle-C-g (...)
>       (when registered-many-C-g
>         (setq num-C-g-presses (1+ num-C-g-presses))
>         (when (> num-C-g-presses 3)
>           (funcall registered-many-C-g-callback))))
>
> > and font-locking code would be roughly
> >
> >     (let ((registered-many-C-g-callback (lambda () (setq font-lock-mode nil))))
> >       ; call fontification function, process its result etc.
> >       )
>
> More like:
>
>     (let ((registered-many-C-g (cons "font-locking..." registered-many-C-g)))
>       ; call fontification function, process its result etc.
>       )
>
> But, yes, maybe it would provide a callback to disable font-lock-mode,
> or just catch the `quit` signal and disable font-lock-mode.
>
> > `count-C-g-presses' to t.  The idea is to not invoke the "abort"
> > callback immediately when the 3rd C-g is pressed, but instead let the
> > iteration finish gracefully and _then_ let the higher-level code
>
> Your bounds-extension code could re-bind registered-many-C-g-callback to
> do something less drastic than disable font-lock-mode (or exit brutally
> from font-lock).
>
>
>         Stefan
>