GNU bug report logs - #75949
kill-buffer-quit-windows breaks kill-buffer-hook

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 75949 in the body.
You can then email your comments to 75949 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: kill-buffer-quit-windows breaks kill-buffer-hook
Date: Thu, 30 Jan 2025 19:04:09 +0200

Please try to evaluate in the *scratch* buffer:

(progn
  (setopt kill-buffer-quit-windows t)
  (view-echo-area-messages)
  (pop-to-buffer-same-window (get-buffer-create "foo"))
  (add-hook 'kill-buffer-hook (lambda () (message "%S" (current-buffer))))
  (add-hook 'kill-buffer-hook (lambda () (kill-buffer (get-buffer "bar")))
	    nil t)
  (display-buffer (get-buffer-create "bar"))
  (switch-to-buffer (get-buffer "*scratch*"))
  (kill-buffer (get-buffer "foo")))

Then observe in the *Messages* buffer that the hook 'kill-buffer-hook'
is run in the *scratch* buffer that is never killed.

Afterwards try to set kill-buffer-quit-windows back to the default value nil,
and evaluate again.  Then everything is correct.

It seems that kill-buffer-quit-windows somehow messes up the current buffer
while running 'kill-buffer-hook'.

This is a distilled test case from the real problem
in treesit--explorer-kill-explorer-buffer
that prevents (add-hook 'kill-buffer-hook #'save-place-to-alist)
from saving places in the right buffer.

Message #8 received at 75949 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>, 75949 <at> debbugs.gnu.org
Subject: Re: bug#75949: kill-buffer-quit-windows breaks kill-buffer-hook
Date: Fri, 31 Jan 2025 09:49:48 +0100

> Then observe in the *Messages* buffer that the hook 'kill-buffer-hook'
> is run in the *scratch* buffer that is never killed.
>
> Afterwards try to set kill-buffer-quit-windows back to the default value nil,
> and evaluate again.  Then everything is correct.
>
> It seems that kill-buffer-quit-windows somehow messes up the current buffer
> while running 'kill-buffer-hook'.

I checked in a fix.  In general, no function on 'kill-buffer-hook' can
be sure that the buffer to be killed is current because another function
on that hook might have changed the current buffer (or even killed it)
before.  That promise, like similar ones in
'window-configuration-change-hook', should never have been made.

An appropriate fix could be possibly made in run_hook_with_args.  But
that might change behaviors that rely on hook-2 running after hook-1
changed the current buffer.  Ideally, we'd have 'kill-buffer-functions'
with the buffer to be killed as sole argument (and the function on the
hook would still have to check whether that buffer is alive).

Thanks, martin

Message #11 received at 75949 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 75949 <at> debbugs.gnu.org
Subject: Re: bug#75949: kill-buffer-quit-windows breaks kill-buffer-hook
Date: Sat, 01 Feb 2025 19:25:47 +0200

close 75949 31.0.50
thanks

>> Then observe in the *Messages* buffer that the hook 'kill-buffer-hook'
>> is run in the *scratch* buffer that is never killed.
>>
>> Afterwards try to set kill-buffer-quit-windows back to the default value nil,
>> and evaluate again.  Then everything is correct.
>>
>> It seems that kill-buffer-quit-windows somehow messes up the current buffer
>> while running 'kill-buffer-hook'.
>
> I checked in a fix.

Thanks, I confirm there is no problem anymore.

> In general, no function on 'kill-buffer-hook' can be sure
> that the buffer to be killed is current because another function
> on that hook might have changed the current buffer (or even killed it)
> before.  That promise, like similar ones in
> 'window-configuration-change-hook', should never have been made.
>
> An appropriate fix could be possibly made in run_hook_with_args.  But
> that might change behaviors that rely on hook-2 running after hook-1
> changed the current buffer.  Ideally, we'd have 'kill-buffer-functions'
> with the buffer to be killed as sole argument (and the function on the
> hook would still have to check whether that buffer is alive).

Since a new argument can't be added, another variant would be at least
to set the current buffer before every call of the hook functions.
I.e. instead of

  (run-hooks 'kill-buffer-hook)

to do something like

  (run-hook-wrapped 'kill-buffer-hook
                    (lambda (fun)
                      (with-current-buffer current-buffer
                        (funcall fun))
                      nil))

But this won't help when buffers are switched inside hook functions.

Message #16 received at 75949 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 75949 <at> debbugs.gnu.org
Subject: Re: bug#75949: kill-buffer-quit-windows breaks kill-buffer-hook
Date: Sun, 2 Feb 2025 09:52:49 +0100

> Since a new argument can't be added, another variant would be at least
> to set the current buffer before every call of the hook functions.
> I.e. instead of
>
>    (run-hooks 'kill-buffer-hook)
>
> to do something like
>
>    (run-hook-wrapped 'kill-buffer-hook
>                      (lambda (fun)
>                        (with-current-buffer current-buffer
>                          (funcall fun))
>                        nil))
>
> But this won't help when buffers are switched inside hook functions.

The problem is that we run hooks completely in C so wrapping is at least
not that simple.  Whatever it is, nesting 'kill-buffer-hook' as in

 (add-hook 'kill-buffer-hook (lambda () (kill-buffer (get-buffer "bar")))
	    nil t)

is not really recommended.

martin

Message #19 received at 75949 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 75949 <at> debbugs.gnu.org
Subject: Re: bug#75949: kill-buffer-quit-windows breaks kill-buffer-hook
Date: Mon, 03 Feb 2025 09:52:47 +0200

> Whatever it is, nesting 'kill-buffer-hook' as in
>
>  (add-hook 'kill-buffer-hook (lambda () (kill-buffer (get-buffer "bar")))
> 	    nil t)
>
> is not really recommended.

What is a recommended way to do this?  This is a valid use case
when killing one buffer requires also killing a dependent buffer.

Message #22 received at 75949 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Juri Linkov <juri <at> linkov.net>
Cc: 75949 <at> debbugs.gnu.org
Subject: Re: bug#75949: kill-buffer-quit-windows breaks kill-buffer-hook
Date: Mon, 3 Feb 2025 09:33:45 +0100

>> Whatever it is, nesting 'kill-buffer-hook' as in
>>
>>   (add-hook 'kill-buffer-hook (lambda () (kill-buffer (get-buffer "bar")))
>> 	    nil t)
>>
>> is not really recommended.
>
> What is a recommended way to do this?  This is a valid use case
> when killing one buffer requires also killing a dependent buffer.

Any function on any hook that changes the corresponding infrastructure
should preserve the state around any such change.  The basic values that
should remain unchanged are the selected window, the selected window of
each frame and the current buffer (unless the function deletes them).

martin

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.