bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Carsten Bormann]

Killing an nXML buffer sometimes leads to a repeated occurrence of the error:

Error in timer: (error "Selecting deleted buffer")

More detail:

Debugger entered--Lisp error: (error "Selecting deleted buffer")
  rng-validate-while-idle(#<killed buffer>)
  apply(rng-validate-while-idle #<killed buffer>)
  byte-code("r\301\b\302H\b\303H\"\210)\301\207" [timer apply 5 6] 4)
  timer-event-handler([t 0 1 500000 t rng-validate-while-idle (#<killed buffer>) idle 0])

I don't have a detailed procedure for reproducing this, yet.

At least, rng-validate-while-idle should kill the timer if the buffer
has been killed.

In GNU Emacs 24.3.50.1 (x86_64-apple-darwin, NS apple-appkit-1038.36)
 of 2013-03-17 on bob.porkrind.org
Bzr revision: 112062 jay.p.belanger@gmail.com-20130317001909-prx18039wq0qmiv4
Windowing system distributor `Apple', version 10.3.1187
Configured using:
 `configure --host=x86_64-apple-darwin --build=i686-apple-darwin
 --with-ns'

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-19 23:35 +0800, Carsten Bormann wrote:
> Killing an nXML buffer sometimes leads to a repeated occurrence of the error:
>
> Error in timer: (error "Selecting deleted buffer")

I have seen this error too. Would you be able to find a way to reproduce
it so that we can understand and fix it with more confidence? Thanks in
advance.

Leo

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Stefan Monnier]

> Debugger entered--Lisp error: (error "Selecting deleted buffer")
>   rng-validate-while-idle(#<killed buffer>)
>   apply(rng-validate-while-idle #<killed buffer>)
>   byte-code("r\301\b\302H\b\303H\"\210)\301\207" [timer apply 5 6] 4)
>   timer-event-handler([t 0 1 500000 t rng-validate-while-idle (#<killed buffer>) idle 0])

Right, rng-validate-while-idle needs to check that its buffer is
still live.  Ideally it should also disable the corresponding timer if
the buffer died.
Could someone take care of it?


        Stefan

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-20 04:51 +0800, Stefan Monnier wrote:
> Right, rng-validate-while-idle needs to check that its buffer is
> still live.  Ideally it should also disable the corresponding timer if
> the buffer died.
> Could someone take care of it?

Last time I checked the timer is supposed to be removed after buffer
being killed. So this fix is not ideal but it gives us a hint that a
recipe might involve killing the buffer.

Leo

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Stefan Monnier]

>> Right, rng-validate-while-idle needs to check that its buffer is
>> still live.  Ideally it should also disable the corresponding timer if
>> the buffer died.
>> Could someone take care of it?
> Last time I checked the timer is supposed to be removed after buffer
> being killed.  So this fix is not ideal but it gives us a hint that a
> recipe might involve killing the buffer.

Yes, clearly a recipe would require killing the buffer in such a way
that the timer is not canceled.  While we may want to try and fix the
hole that lets this happen, I think it's still worthwhile to defensively
make rng-validate-while-idle check that the buffer is still live, since
the other problem may be anywhere in some unrelated code (potentially
external to Emacs) that let-binds kill-buffer-hook (for example).


        Stefan

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-20 11:29 +0800, Stefan Monnier wrote:
[snipped 8 lines]
> Yes, clearly a recipe would require killing the buffer in such a way
> that the timer is not canceled.  While we may want to try and fix the
> hole that lets this happen, I think it's still worthwhile to defensively
> make rng-validate-while-idle check that the buffer is still live, since
> the other problem may be anywhere in some unrelated code (potentially
> external to Emacs) that let-binds kill-buffer-hook (for example).

I fully agree. I'll see if I can get to the root of the problem while
the OP is finding a recipe. IOW, I'll take care of this bug.

Thanks.
Leo

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-20 12:30 +0800, Leo Liu wrote:
>> Yes, clearly a recipe would require killing the buffer in such a way
>> that the timer is not canceled.  While we may want to try and fix the
>> hole that lets this happen, I think it's still worthwhile to defensively
>> make rng-validate-while-idle check that the buffer is still live, since
>> the other problem may be anywhere in some unrelated code (potentially
>> external to Emacs) that let-binds kill-buffer-hook (for example).
>
> I fully agree. I'll see if I can get to the root of the problem while
> the OP is finding a recipe. IOW, I'll take care of this bug.

One possibility is the buffer is killed while timer-idle-list is
temporarily bound to nil, in this case the timer will be left in the
timer-idle-list and causing this error.

The code in rng-valid.el uses the following idiom to redisplay without
triggering idle timers.

(let ((timer-idle-list nil))
   ....
   (sit-for 0)
   ....)

Is there a way to do this without let-binding timer-idle-list?

Leo

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Stefan Monnier]

> The code in rng-valid.el uses the following idiom to redisplay without
> triggering idle timers.

Why does it need to do that?


        Stefan

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-22 21:36 +0800, Stefan Monnier wrote:
>> The code in rng-valid.el uses the following idiom to redisplay without
>> triggering idle timers.
>
> Why does it need to do that?
>
>
>         Stefan

Does the following make any sense?

(defun rng-validate-while-idle-continue-p ()
  ;; input-pending-p and sit-for run timers that are
  ;; ripe.  Binding timer-idle-list to nil prevents
  ;; this.  If we don't do this, then any ripe timers
  ;; will get run, and we won't get any chance to
  ;; validate until Emacs becomes idle again or until
  ;; the other lower priority timers finish (which
  ;; can take a very long time in the case of
  ;; jit-lock).
  (let ((timer-idle-list nil))
    (and (not (input-pending-p))
	 ;; Fake rng-validate-up-to-date-end so that the mode line
	 ;; shows progress.  Also use this to save point.
	 (let ((rng-validate-up-to-date-end (point)))
	   (goto-char rng-validate-display-point)
	   (when (not rng-validate-display-modified-p)
	     (restore-buffer-modified-p nil))
	   (force-mode-line-update)
	   (let ((continue (sit-for 0)))
	     (goto-char rng-validate-up-to-date-end)
	     continue)))))

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Stefan Monnier]

>>> The code in rng-valid.el uses the following idiom to redisplay without
>>> triggering idle timers.
>> Why does it need to do that?
> Does the following make any sense?

No, I don't understand what problem this is trying to avoid.
If noone understands this hack, I suggest to get rid of this
let-binding, and if/when the corresponding problem ever appears, we'll
see about fixing it some other way.


        Stefan

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-23 01:34 +0800, Stefan Monnier wrote:
> No, I don't understand what problem this is trying to avoid.
> If noone understands this hack, I suggest to get rid of this
> let-binding, and if/when the corresponding problem ever appears, we'll
> see about fixing it some other way.

I get rid of it for now.

Carsten, will you be able to test out the new rng-valid.el¹²? Thanks.

Footnotes: 
¹  http://repo.or.cz/w/emacs.git/blob_plain/HEAD:/lisp/nxml/rng-valid.el
²  http://repo.or.cz/w/emacs.git/blob_plain/HEAD:/lisp/nxml/rng-nxml.el

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Carsten Bormann]

On Mar 23, 2013, at 02:47, Leo Liu <sdl.web@gmail.com> wrote:

> On 2013-03-23 01:34 +0800, Stefan Monnier wrote:
>> No, I don't understand what problem this is trying to avoid.
>> If noone understands this hack, I suggest to get rid of this
>> let-binding, and if/when the corresponding problem ever appears, we'll
>> see about fixing it some other way.
> 
> I get rid of it for now.
> 
> Carsten, will you be able to test out the new rng-valid.el¹²? Thanks.
> 
> Footnotes: 
> ¹  http://repo.or.cz/w/emacs.git/blob_plain/HEAD:/lisp/nxml/rng-valid.el
> ²  http://repo.or.cz/w/emacs.git/blob_plain/HEAD:/lisp/nxml/rng-nxml.el

Well, how do you test that...
I played around with background-validating an XML file, scrolling while that happens, and that seems to work.
I don't use stealthy jit-lock etc., though, and my XML files are of moderate size (couple hundred kB), so maybe I'm not in the target group addressed by the original hack.

It seems really solving the problem the code was trying to address would require having a priority scheme for idle timer work: Validation happens in idle time but may be higher priority than other idle timer work.  Another way of blocking other idle timer work while doing sit-for/input-pending-p would, of course, also help; this might need exposing the READABLE_EVENTS_DO_TIMERS_NOW flag from input-pending-p and sit-for.

Grüße, Carsten

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-23 15:54 +0800, Carsten Bormann wrote:
> Well, how do you test that...
> I played around with background-validating an XML file, scrolling
> while that happens, and that seems to work.
> I don't use stealthy jit-lock etc., though, and my XML files are of
> moderate size (couple hundred kB), so maybe I'm not in the target
> group addressed by the original hack.
>
> It seems really solving the problem the code was trying to address
> would require having a priority scheme for idle timer work: Validation
> happens in idle time but may be higher priority than other idle timer
> work. Another way of blocking other idle timer work while doing
> sit-for/input-pending-p would, of course, also help; this might need
> exposing the READABLE_EVENTS_DO_TIMERS_NOW flag from input-pending-p
> and sit-for.
>
> Grüße, Carsten

OK thanks for the information. I use nxml even less. I only use it to
edit plist files on OSX which are small in general. So I'll assume it is
good enough.

Leo

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

On 2013-03-20 11:29 +0800, Stefan Monnier wrote:
> I think it's still worthwhile to defensively make
> rng-validate-while-idle check that the buffer is still live, since the
> other problem may be anywhere in some unrelated code (potentially
> external to Emacs) that let-binds kill-buffer-hook (for example).

Let-binding kill-buffer-hook is also a source of problems like this bug
report. There are three places in emacs that do this:

  international/mule.el
  emacs-lisp/edebug.el
  gnus/mml.el

Looks like a really bad idea.

Most uses of kill-buffer-hook are cleanup routines. Let-bind the hook to
nil has a high probability of leaving things in an inconsistent state.

I think we should find a way to fix the three occurrences in emacs and
put a note in kill-buffer-hook's doc-string.

Leo

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Stefan Monnier]

> Let-binding kill-buffer-hook is also a source of problems like this bug
> report.

Let-binding a hook is generally a bad idea, yes.  It's even difficult to
ensure that it actually does what one intended.


        Stefan

bug#13999: 24.3.50; rng-validate-while-idle -> (error "Selecting deleted buffer")

[Leo Liu]

Fixed in trunk.