unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Should (buffer-list) ever return killed buffers?
@ 2021-05-23 23:14 Sergey Organov
  2021-05-23 23:41 ` Stefan Monnier
  2021-05-23 23:42 ` Clément Pit-Claudel
  0 siblings, 2 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-23 23:14 UTC (permalink / raw)
  To: emacs-devel

Hello!

I'm trying to fix a problem where `desktop-clear` errors-out because
(buffer-list) contains killed buffer.

Somehow it happens when `desktop-clear` is called from a timer and there
is *Info* buffer out there, and apparently the one that is killed is

"#<buffer  *info tag table*>"

I'd like to know if this is a bug in `destop-clear`, `buffer-list`,
info, or elsewhere?

Thanks,
-- Sergey Organov



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:14 Should (buffer-list) ever return killed buffers? Sergey Organov
@ 2021-05-23 23:41 ` Stefan Monnier
  2021-05-23 23:58   ` Sergey Organov
                     ` (2 more replies)
  2021-05-23 23:42 ` Clément Pit-Claudel
  1 sibling, 3 replies; 31+ messages in thread
From: Stefan Monnier @ 2021-05-23 23:41 UTC (permalink / raw)
  To: Sergey Organov; +Cc: emacs-devel

> Somehow it happens when `desktop-clear` is called from a timer and there
> is *Info* buffer out there, and apparently the one that is killed is
>
> "#<buffer  *info tag table*>"
>
> I'd like to know if this is a bug in `destop-clear`, `buffer-list`,
> info, or elsewhere?

I strongly suspect that the problem goes as follows:
- buffer-list returns a list of buffers that are all live (i.e. no bug
  there).
- while processing that list, some of its buffers die.
So I think the bug is in `desktop-clear` which should skip buffers that
have died between the call to `buffer-list` and the moment we get to
process them.


        Stefan "who hasn't looked at the code"




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:14 Should (buffer-list) ever return killed buffers? Sergey Organov
  2021-05-23 23:41 ` Stefan Monnier
@ 2021-05-23 23:42 ` Clément Pit-Claudel
  2021-05-23 23:55   ` Sergey Organov
  1 sibling, 1 reply; 31+ messages in thread
From: Clément Pit-Claudel @ 2021-05-23 23:42 UTC (permalink / raw)
  To: emacs-devel

On 5/23/21 7:14 PM, Sergey Organov wrote:
> Hello!
> 
> I'm trying to fix a problem where `desktop-clear` errors-out because
> (buffer-list) contains killed buffer.

Could it be that the actions you take on one buffer while iterating on (buffer-list) cause another buffer later in the list to be killed? 



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:42 ` Clément Pit-Claudel
@ 2021-05-23 23:55   ` Sergey Organov
  2021-05-23 23:56     ` Clément Pit-Claudel
  0 siblings, 1 reply; 31+ messages in thread
From: Sergey Organov @ 2021-05-23 23:55 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: emacs-devel

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> On 5/23/21 7:14 PM, Sergey Organov wrote:
>> Hello!
>> 
>> I'm trying to fix a problem where `desktop-clear` errors-out because
>> (buffer-list) contains killed buffer.
>
> Could it be that the actions you take on one buffer while iterating on
> (buffer-list) cause another buffer later in the list to be killed?

This is relevant code snippet from "desktop.el":

    (dolist (buffer (buffer-list))
      (let ((bufname (buffer-name buffer)))
	(unless (or (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
		    (string-match-p preserve-regexp bufname))
	  (kill-buffer buffer)))))


I fail to see how I can take any actions between its calls to (buffer-list)
and (aref bufname 0), that errors-out of bufname being nil.

Ah, apparently killing one buffer kills another... but why it happens
only when called from timer? Mystery.

Thanks,
-- Sergey Organov



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:55   ` Sergey Organov
@ 2021-05-23 23:56     ` Clément Pit-Claudel
  0 siblings, 0 replies; 31+ messages in thread
From: Clément Pit-Claudel @ 2021-05-23 23:56 UTC (permalink / raw)
  To: Sergey Organov; +Cc: emacs-devel

On 5/23/21 7:55 PM, Sergey Organov wrote:
> Ah, apparently killing one buffer kills another...

Precisely.  We ran into the exact same bug in Flycheck a while ago :)



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:41 ` Stefan Monnier
@ 2021-05-23 23:58   ` Sergey Organov
  2021-05-24  3:32     ` Stefan Monnier
  2021-05-24 14:15     ` martin rudalics
  2021-05-24 13:41   ` [PATCH] " Sergey Organov
  2021-07-22 11:27   ` Sergey Organov
  2 siblings, 2 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-23 23:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Somehow it happens when `desktop-clear` is called from a timer and there
>> is *Info* buffer out there, and apparently the one that is killed is
>>
>> "#<buffer  *info tag table*>"
>>
>> I'd like to know if this is a bug in `destop-clear`, `buffer-list`,
>> info, or elsewhere?
>
> I strongly suspect that the problem goes as follows:
> - buffer-list returns a list of buffers that are all live (i.e. no bug
>   there).
> - while processing that list, some of its buffers die.
> So I think the bug is in `desktop-clear` which should skip buffers that
> have died between the call to `buffer-list` and the moment we get to
> process them.

Yep, looks like this. The only question then is why didn't it ever fail
for me before, for about 10 years, and started to fail only recently,
despite "desktop.el" is like that for ages, as far as I can see. And how
running from a timer could be involved?

Thanks,
-- Sergey Organov



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:58   ` Sergey Organov
@ 2021-05-24  3:32     ` Stefan Monnier
  2021-05-24 14:15     ` martin rudalics
  1 sibling, 0 replies; 31+ messages in thread
From: Stefan Monnier @ 2021-05-24  3:32 UTC (permalink / raw)
  To: Sergey Organov; +Cc: emacs-devel

>> - while processing that list, some of its buffers die.
>> So I think the bug is in `desktop-clear` which should skip buffers that
>> have died between the call to `buffer-list` and the moment we get to
>> process them.
>
> Yep, looks like this. The only question then is why didn't it ever fail
> for me before, for about 10 years, and started to fail only recently,
> despite "desktop.el" is like that for ages, as far as I can see. And how
> running from a timer could be involved?

It all depends on what's going on the loop, the order of buffers in the
list, etc.... it's not hard to imagine a slight innocent change
somewhere introducing this corner case where it didn't show up earlier.


        Stefan




^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:41 ` Stefan Monnier
  2021-05-23 23:58   ` Sergey Organov
@ 2021-05-24 13:41   ` Sergey Organov
  2021-05-24 14:04     ` Tassilo Horn
  2021-05-24 14:27     ` Eli Zaretskii
  2021-07-22 11:27   ` Sergey Organov
  2 siblings, 2 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 13:41 UTC (permalink / raw)
  To: emacs-devel; +Cc: Eli Zaretskii, Clément Pit-Claudel, Stefan Monnier

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Somehow it happens when `desktop-clear` is called from a timer and there
>> is *Info* buffer out there, and apparently the one that is killed is
>>
>> "#<buffer  *info tag table*>"
>>
>> I'd like to know if this is a bug in `destop-clear`, `buffer-list`,
>> info, or elsewhere?
>
> I strongly suspect that the problem goes as follows:
> - buffer-list returns a list of buffers that are all live (i.e. no bug
>   there).
> - while processing that list, some of its buffers die.
> So I think the bug is in `desktop-clear` which should skip buffers that
> have died between the call to `buffer-list` and the moment we get to
> process them.

OK, here is a patch that fixes the issue:

    lisp/desktop.el: check for killed buffers in desktop-clear

diff --git a/lisp/desktop.el b/lisp/desktop.el
index fb7c6c79a1..20d9e0f268 100644
--- a/lisp/desktop.el
+++ b/lisp/desktop.el
@@ -706,7 +706,8 @@ if different)."
                                  "\\)\\'")))
     (dolist (buffer (buffer-list))
       (let ((bufname (buffer-name buffer)))
-	(unless (or (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
+	(unless (or (nill bufname)
+		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
 		    (string-match-p preserve-regexp bufname))
 	  (kill-buffer buffer)))))
   (delete-other-windows)

Thanks,
-- Sergey Organov



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 13:41   ` [PATCH] " Sergey Organov
@ 2021-05-24 14:04     ` Tassilo Horn
  2021-05-24 14:25       ` Sergey Organov
  2021-05-24 14:27     ` Eli Zaretskii
  1 sibling, 1 reply; 31+ messages in thread
From: Tassilo Horn @ 2021-05-24 14:04 UTC (permalink / raw)
  To: emacs-devel

Sergey Organov <sorganov@gmail.com> writes:

> OK, here is a patch that fixes the issue:
>
>     lisp/desktop.el: check for killed buffers in desktop-clear
>
> diff --git a/lisp/desktop.el b/lisp/desktop.el
> index fb7c6c79a1..20d9e0f268 100644
> --- a/lisp/desktop.el
> +++ b/lisp/desktop.el
> @@ -706,7 +706,8 @@ if different)."
>                                   "\\)\\'")))
>      (dolist (buffer (buffer-list))
>        (let ((bufname (buffer-name buffer)))
> -	(unless (or (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
> +	(unless (or (nill bufname)
                     ^^^^

That should most probably be `null'.

> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>  		    (string-match-p preserve-regexp bufname))
>  	  (kill-buffer buffer)))))
>    (delete-other-windows)

Bye,
Tassilo



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:58   ` Sergey Organov
  2021-05-24  3:32     ` Stefan Monnier
@ 2021-05-24 14:15     ` martin rudalics
  2021-05-24 14:23       ` Sergey Organov
                         ` (2 more replies)
  1 sibling, 3 replies; 31+ messages in thread
From: martin rudalics @ 2021-05-24 14:15 UTC (permalink / raw)
  To: Sergey Organov, Stefan Monnier; +Cc: emacs-devel

 >> I strongly suspect that the problem goes as follows:
 >> - buffer-list returns a list of buffers that are all live (i.e. no bug
 >>    there).

I strongly doubt that.  For me the reason is that `buffer-list' runs
FOR_EACH_TAIL_INTERNAL with third argument true which may quit.  The
earlier mentioned "And how running from a timer could be involved?"
should explain what happens then.

 >> - while processing that list, some of its buffers die.
 >> So I think the bug is in `desktop-clear` which should skip buffers that
 >> have died between the call to `buffer-list` and the moment we get to
 >> process them.

We should fix `buffer-list' appropriately.  IIUC it's broken anyway with
a non-nil FRAME argument - it nowhere checks whether the buffers on
FRAME's buffer list and buried buffer list are live.

 > +	(unless (or (nill bufname)
                      ^^^^

martin



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 14:15     ` martin rudalics
@ 2021-05-24 14:23       ` Sergey Organov
  2021-05-24 16:31       ` Stefan Monnier
  2021-05-24 20:27       ` Sergey Organov
  2 siblings, 0 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 14:23 UTC (permalink / raw)
  To: martin rudalics; +Cc: Stefan Monnier, emacs-devel

martin rudalics <rudalics@gmx.at> writes:

>>> I strongly suspect that the problem goes as follows:
>>> - buffer-list returns a list of buffers that are all live (i.e. no bug
>>>    there).
>
> I strongly doubt that.  For me the reason is that `buffer-list' runs
> FOR_EACH_TAIL_INTERNAL with third argument true which may quit.  The
> earlier mentioned "And how running from a timer could be involved?"
> should explain what happens then.
>
>>> - while processing that list, some of its buffers die.
>>> So I think the bug is in `desktop-clear` which should skip buffers that
>>> have died between the call to `buffer-list` and the moment we get to
>>> process them.
>
> We should fix `buffer-list' appropriately.  IIUC it's broken anyway with
> a non-nil FRAME argument - it nowhere checks whether the buffers on
> FRAME's buffer list and buried buffer list are live.
>
>> +	(unless (or (nill bufname)
>                    ^^^^

Yep, that's needs to be (null bufname), obviously. Messed it when
re-typed code from another PC where I've tested it, sorry!

-- Sergey



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 14:04     ` Tassilo Horn
@ 2021-05-24 14:25       ` Sergey Organov
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 14:25 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: emacs-devel

Tassilo Horn <tsdh@gnu.org> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> OK, here is a patch that fixes the issue:
>>
>>     lisp/desktop.el: check for killed buffers in desktop-clear
>>
>> diff --git a/lisp/desktop.el b/lisp/desktop.el
>> index fb7c6c79a1..20d9e0f268 100644
>> --- a/lisp/desktop.el
>> +++ b/lisp/desktop.el
>> @@ -706,7 +706,8 @@ if different)."
>>                                   "\\)\\'")))
>>      (dolist (buffer (buffer-list))
>>        (let ((bufname (buffer-name buffer)))
>> -	(unless (or (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>> +	(unless (or (nill bufname)
>                      ^^^^
>
> That should most probably be `null'.

Yep, sorry for messing it while re-typing from working code!

Thanks,
-- Sergey



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 13:41   ` [PATCH] " Sergey Organov
  2021-05-24 14:04     ` Tassilo Horn
@ 2021-05-24 14:27     ` Eli Zaretskii
  2021-05-24 14:50       ` martin rudalics
  1 sibling, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2021-05-24 14:27 UTC (permalink / raw)
  To: Sergey Organov; +Cc: cpitclaudel, monnier, emacs-devel

> From: Sergey Organov <sorganov@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,
>     Stefan Monnier <monnier@iro.umontreal.ca>,
>     Clément Pit-Claudel <cpitclaudel@gmail.com> 
> Date: Mon, 24 May 2021 16:41:07 +0300
> 
> diff --git a/lisp/desktop.el b/lisp/desktop.el
> index fb7c6c79a1..20d9e0f268 100644
> --- a/lisp/desktop.el
> +++ b/lisp/desktop.el
> @@ -706,7 +706,8 @@ if different)."
>                                   "\\)\\'")))
>      (dolist (buffer (buffer-list))
>        (let ((bufname (buffer-name buffer)))
> -	(unless (or (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
> +	(unless (or (nill bufname)
> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>  		    (string-match-p preserve-regexp bufname))
>  	  (kill-buffer buffer)))))
>    (delete-other-windows)

AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
what exactly does this fix?  (Apologies if I'm missing something: I
cannot say that I've read all the discussions in this thread to the
last detail.)



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 14:27     ` Eli Zaretskii
@ 2021-05-24 14:50       ` martin rudalics
  2021-05-24 15:05         ` Eli Zaretskii
  2021-05-24 15:14         ` Sergey Organov
  0 siblings, 2 replies; 31+ messages in thread
From: martin rudalics @ 2021-05-24 14:50 UTC (permalink / raw)
  To: Eli Zaretskii, Sergey Organov; +Cc: cpitclaudel, monnier, emacs-devel

 >> +	(unless (or (nill bufname)
 >> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
 >>   		    (string-match-p preserve-regexp bufname))
 >>   	  (kill-buffer buffer)))))
 >>     (delete-other-windows)
 >
 > AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
 > what exactly does this fix?  (Apologies if I'm missing something: I
 > cannot say that I've read all the discussions in this thread to the
 > last detail.)

Sergey never told us but it's likely `aref' choking on nil.

martin



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 14:50       ` martin rudalics
@ 2021-05-24 15:05         ` Eli Zaretskii
  2021-05-24 15:32           ` Sergey Organov
  2021-05-24 16:04           ` martin rudalics
  2021-05-24 15:14         ` Sergey Organov
  1 sibling, 2 replies; 31+ messages in thread
From: Eli Zaretskii @ 2021-05-24 15:05 UTC (permalink / raw)
  To: martin rudalics; +Cc: cpitclaudel, sorganov, monnier, emacs-devel

> Cc: cpitclaudel@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 24 May 2021 16:50:48 +0200
> 
>  >> +	(unless (or (nill bufname)
>  >> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>  >>   		    (string-match-p preserve-regexp bufname))
>  >>   	  (kill-buffer buffer)))))
>  >>     (delete-other-windows)
>  >
>  > AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
>  > what exactly does this fix?  (Apologies if I'm missing something: I
>  > cannot say that I've read all the discussions in this thread to the
>  > last detail.)
> 
> Sergey never told us but it's likely `aref' choking on nil.

Then why not use buffer-live-p?



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 14:50       ` martin rudalics
  2021-05-24 15:05         ` Eli Zaretskii
@ 2021-05-24 15:14         ` Sergey Organov
  1 sibling, 0 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 15:14 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, emacs-devel, cpitclaudel, monnier

martin rudalics <rudalics@gmx.at> writes:

>>> +	(unless (or (nill bufname)
>>> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>>>   		    (string-match-p preserve-regexp bufname))
>>>   	  (kill-buffer buffer)))))
>>>     (delete-other-windows)
>>
>> AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
>> what exactly does this fix?  (Apologies if I'm missing something: I
>> cannot say that I've read all the discussions in this thread to the
>> last detail.)
>
> Sergey never told us but it's likely `aref' choking on nil.

Exactly. Sorry for not saying from the beginning!

-- Sergey



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 15:05         ` Eli Zaretskii
@ 2021-05-24 15:32           ` Sergey Organov
  2021-05-24 16:07             ` [PATCH] " Philipp
  2021-05-24 16:25             ` [PATCH] " Eli Zaretskii
  2021-05-24 16:04           ` martin rudalics
  1 sibling, 2 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 15:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: martin rudalics, cpitclaudel, monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: cpitclaudel@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> From: martin rudalics <rudalics@gmx.at>
>> Date: Mon, 24 May 2021 16:50:48 +0200
>> 
>>  >> +	(unless (or (nill bufname)
>>  >> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>>  >>   		    (string-match-p preserve-regexp bufname))
>>  >>   	  (kill-buffer buffer)))))
>>  >>     (delete-other-windows)
>>  >
>>  > AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
>>  > what exactly does this fix?  (Apologies if I'm missing something: I
>>  > cannot say that I've read all the discussions in this thread to the
>>  > last detail.)
>> 
>> Sergey never told us but it's likely `aref' choking on nil.
>
> Then why not use buffer-live-p?

Cause aref is choking on bufname being nil, I think. What if
representation of killed buffers change?

Thanks,
-- Sergey



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 15:05         ` Eli Zaretskii
  2021-05-24 15:32           ` Sergey Organov
@ 2021-05-24 16:04           ` martin rudalics
  2021-05-24 16:30             ` Eli Zaretskii
  1 sibling, 1 reply; 31+ messages in thread
From: martin rudalics @ 2021-05-24 16:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: cpitclaudel, sorganov, monnier, emacs-devel

 >> Sergey never told us but it's likely `aref' choking on nil.
 >
 > Then why not use buffer-live-p?

The bug is in `buffer-list' which should not return dead buffers.  It
should either bind Qinhibit_quit around all Lisp routines it calls or
better use _no_quit functions for handling lists that are known to be
non-circular like Vbuffer_list.

Note in this regard that even the `reorder-buffer-list' from the Elisp
manual will fail in the same way because it uses FOR_EACH_TAIL_INTERNAL
too.

martin



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Should (buffer-list) ever return killed buffers?
  2021-05-24 15:32           ` Sergey Organov
@ 2021-05-24 16:07             ` Philipp
  2021-05-24 18:11               ` Sergey Organov
  2021-05-24 18:31               ` Sergey Organov
  2021-05-24 16:25             ` [PATCH] " Eli Zaretskii
  1 sibling, 2 replies; 31+ messages in thread
From: Philipp @ 2021-05-24 16:07 UTC (permalink / raw)
  To: Sergey Organov
  Cc: martin rudalics, Eli Zaretskii, emacs-devel, cpitclaudel, monnier



> Am 24.05.2021 um 17:32 schrieb Sergey Organov <sorganov@gmail.com>:
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>>> Cc: cpitclaudel@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>> From: martin rudalics <rudalics@gmx.at>
>>> Date: Mon, 24 May 2021 16:50:48 +0200
>>> 
>>>>> +	(unless (or (nill bufname)
>>>>> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>>>>>  		    (string-match-p preserve-regexp bufname))
>>>>>  	  (kill-buffer buffer)))))
>>>>>    (delete-other-windows)
>>>> 
>>>> AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
>>>> what exactly does this fix?  (Apologies if I'm missing something: I
>>>> cannot say that I've read all the discussions in this thread to the
>>>> last detail.)
>>> 
>>> Sergey never told us but it's likely `aref' choking on nil.
>> 
>> Then why not use buffer-live-p?
> 
> Cause aref is choking on bufname being nil, I think. What if
> representation of killed buffers change?

That won't happen.  The ELisp manual guarantees that "[t]he ‘buffer-name’ of a buffer is ‘nil’ if, and only if, the buffer is killed."


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 15:32           ` Sergey Organov
  2021-05-24 16:07             ` [PATCH] " Philipp
@ 2021-05-24 16:25             ` Eli Zaretskii
  2021-05-24 18:09               ` Sergey Organov
  1 sibling, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2021-05-24 16:25 UTC (permalink / raw)
  To: Sergey Organov; +Cc: rudalics, cpitclaudel, monnier, emacs-devel

> From: Sergey Organov <sorganov@gmail.com>
> Cc: martin rudalics <rudalics@gmx.at>,  cpitclaudel@gmail.com,
>   monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 24 May 2021 18:32:18 +0300
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Then why not use buffer-live-p?
> 
> Cause aref is choking on bufname being nil, I think. What if
> representation of killed buffers change?

Whatever the representation of killed buffers, a buffer with a name of
nil will never be alive.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 16:04           ` martin rudalics
@ 2021-05-24 16:30             ` Eli Zaretskii
  2021-05-24 19:01               ` Stefan Monnier
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2021-05-24 16:30 UTC (permalink / raw)
  To: martin rudalics; +Cc: cpitclaudel, sorganov, monnier, emacs-devel

> Cc: cpitclaudel@gmail.com, sorganov@gmail.com, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 24 May 2021 18:04:25 +0200
> 
> The bug is in `buffer-list' which should not return dead buffers.

I don't think it's practical to guarantee that, given the myriad of
hooks we have.



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 14:15     ` martin rudalics
  2021-05-24 14:23       ` Sergey Organov
@ 2021-05-24 16:31       ` Stefan Monnier
  2021-05-24 17:05         ` martin rudalics
  2021-05-24 20:27       ` Sergey Organov
  2 siblings, 1 reply; 31+ messages in thread
From: Stefan Monnier @ 2021-05-24 16:31 UTC (permalink / raw)
  To: martin rudalics; +Cc: Sergey Organov, emacs-devel

>>> I strongly suspect that the problem goes as follows:
>>> - buffer-list returns a list of buffers that are all live (i.e. no bug
>>>    there).
> I strongly doubt that.  For me the reason is that `buffer-list' runs
> FOR_EACH_TAIL_INTERNAL with third argument true which may quit.  The
> earlier mentioned "And how running from a timer could be involved?"
> should explain what happens then.

I don't see how it explain anything.  The fact that it may quit doesn't
imply it can run timers or run arbitrary code which could lead to the
return value containing dead buffers.

> We should fix `buffer-list' appropriately.  IIUC it's broken anyway with
> a non-nil FRAME argument - it nowhere checks whether the buffers on
> FRAME's buffer list and buried buffer list are live.

I never use the FRAME arg to it, so I can't speak for the behavior of
that use-case.  If it currently returns dead buffers, we should fix that.


        Stefan




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 16:31       ` Stefan Monnier
@ 2021-05-24 17:05         ` martin rudalics
  2021-05-24 19:55           ` Stefan Monnier
  0 siblings, 1 reply; 31+ messages in thread
From: martin rudalics @ 2021-05-24 17:05 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Sergey Organov, emacs-devel

 > I don't see how it explain anything.  The fact that it may quit doesn't
 > imply it can run timers or run arbitrary code which could lead to the
 > return value containing dead buffers.

Even within `while-no-input'?

martin



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 16:25             ` [PATCH] " Eli Zaretskii
@ 2021-05-24 18:09               ` Sergey Organov
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 18:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rudalics, cpitclaudel, monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Sergey Organov <sorganov@gmail.com>
>> Cc: martin rudalics <rudalics@gmx.at>,  cpitclaudel@gmail.com,
>>   monnier@iro.umontreal.ca,  emacs-devel@gnu.org
>> Date: Mon, 24 May 2021 18:32:18 +0300
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Then why not use buffer-live-p?
>> 
>> Cause aref is choking on bufname being nil, I think. What if
>> representation of killed buffers change?
>
> Whatever the representation of killed buffers, a buffer with a name of
> nil will never be alive.

Is there a fundamental law that prohibits this? ;-)

That said, I believe you guys know better how to handle this, I just
provided raw approximation for the fix, for convenience.

Thanks,
-- Sergey Organov



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Should (buffer-list) ever return killed buffers?
  2021-05-24 16:07             ` [PATCH] " Philipp
@ 2021-05-24 18:11               ` Sergey Organov
  2021-05-24 18:31               ` Sergey Organov
  1 sibling, 0 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 18:11 UTC (permalink / raw)
  To: Philipp; +Cc: martin rudalics, Eli Zaretskii, emacs-devel, cpitclaudel, monnier

Philipp <p.stephani2@gmail.com> writes:

>> Am 24.05.2021 um 17:32 schrieb Sergey Organov <sorganov@gmail.com>:
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>>>> Cc: cpitclaudel@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>>> From: martin rudalics <rudalics@gmx.at>
>>>> Date: Mon, 24 May 2021 16:50:48 +0200
>>>> 
>>>>>> +	(unless (or (nill bufname)
>>>>>> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>>>>>>  		    (string-match-p preserve-regexp bufname))
>>>>>>  	  (kill-buffer buffer)))))
>>>>>>    (delete-other-windows)
>>>>> 
>>>>> AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
>>>>> what exactly does this fix?  (Apologies if I'm missing something: I
>>>>> cannot say that I've read all the discussions in this thread to the
>>>>> last detail.)
>>>> 
>>>> Sergey never told us but it's likely `aref' choking on nil.
>>> 
>>> Then why not use buffer-live-p?
>> 
>> Cause aref is choking on bufname being nil, I think. What if
>> representation of killed buffers change?
>
> That won't happen. The ELisp manual guarantees that "[t]he
> ‘buffer-name’ of a buffer is ‘nil’ if, and only if, the buffer is
> killed."

Probably. I provided a patch as convenience. Whatever fix you guys
come-up with, I'll be happy to use.

Thanks,
-- Sergey Organov




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Should (buffer-list) ever return killed buffers?
  2021-05-24 16:07             ` [PATCH] " Philipp
  2021-05-24 18:11               ` Sergey Organov
@ 2021-05-24 18:31               ` Sergey Organov
  1 sibling, 0 replies; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 18:31 UTC (permalink / raw)
  To: Philipp; +Cc: martin rudalics, Eli Zaretskii, cpitclaudel, monnier, emacs-devel

Philipp <p.stephani2@gmail.com> writes:

>> Am 24.05.2021 um 17:32 schrieb Sergey Organov <sorganov@gmail.com>:
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>>>> Cc: cpitclaudel@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>>> From: martin rudalics <rudalics@gmx.at>
>>>> Date: Mon, 24 May 2021 16:50:48 +0200
>>>> 
>>>>>> +	(unless (or (nill bufname)
>>>>>> +		    (eq (aref bufname 0) ?\s) ;; Don't kill internal buffers
>>>>>>  		    (string-match-p preserve-regexp bufname))
>>>>>>  	  (kill-buffer buffer)))))
>>>>>>    (delete-other-windows)
>>>>> 
>>>>> AFAICT, kill-buffer already is a no-op when the buffer is dead.  So
>>>>> what exactly does this fix?  (Apologies if I'm missing something: I
>>>>> cannot say that I've read all the discussions in this thread to the
>>>>> last detail.)
>>>> 
>>>> Sergey never told us but it's likely `aref' choking on nil.
>>> 
>>> Then why not use buffer-live-p?
>> 
>> Cause aref is choking on bufname being nil, I think. What if
>> representation of killed buffers change?
>
> That won't happen. The ELisp manual guarantees that "[t]he
> ‘buffer-name’ of a buffer is ‘nil’ if, and only if, the buffer is
> killed."

Provide (null bufname) and (buffer-live-p buf) are synonyms, as the
manual claims,the former looks both more convenient and more logical in
this particular instance, as we already have bufname anyway, and the
problem is exactly because it's nil, not because the buffer is killed.

OTOH, killing of zombie buffer is not a problem by itself, so I don't
see why it should be explicitly checked and avoided.

Thanks,
-- Sergey Organov




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 16:30             ` Eli Zaretskii
@ 2021-05-24 19:01               ` Stefan Monnier
  0 siblings, 0 replies; 31+ messages in thread
From: Stefan Monnier @ 2021-05-24 19:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: martin rudalics, cpitclaudel, sorganov, emacs-devel

>> The bug is in `buffer-list' which should not return dead buffers.
> I don't think it's practical to guarantee that,

Actually, I think it does guarantee that currently (at least when
called with no arguments).

> given the myriad of hooks we have.

But none of them can be called during the execution of `buffer-list` so
it's easy to ensure that all the buffers are live.

Of course, that won't solve the problem here where the buffers become
dead subsequently while the code is iterating through the list
previously returned by `buffer-list`.


        Stefan




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 17:05         ` martin rudalics
@ 2021-05-24 19:55           ` Stefan Monnier
  0 siblings, 0 replies; 31+ messages in thread
From: Stefan Monnier @ 2021-05-24 19:55 UTC (permalink / raw)
  To: martin rudalics; +Cc: Sergey Organov, emacs-devel

>> I don't see how it explain anything.  The fact that it may quit doesn't
>> imply it can run timers or run arbitrary code which could lead to the
>> return value containing dead buffers.
> Even within `while-no-input'?

Yes, even with that.  "may quit" means just that: it can be interrupted
in the middle, but if so it won't return any list at all: it will
just quit.  So when it does return, it always returns a list and it has
not been interrupted by any `quit` along the way, and no other ELisp
code can have run in-between either.
AFAIK


        Stefan




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 14:15     ` martin rudalics
  2021-05-24 14:23       ` Sergey Organov
  2021-05-24 16:31       ` Stefan Monnier
@ 2021-05-24 20:27       ` Sergey Organov
  2021-05-25  6:49         ` martin rudalics
  2 siblings, 1 reply; 31+ messages in thread
From: Sergey Organov @ 2021-05-24 20:27 UTC (permalink / raw)
  To: martin rudalics; +Cc: Stefan Monnier, emacs-devel

martin rudalics <rudalics@gmx.at> writes:

>>> I strongly suspect that the problem goes as follows:
>>> - buffer-list returns a list of buffers that are all live (i.e. no bug
>>>    there).
>
> I strongly doubt that.  For me the reason is that `buffer-list' runs
> FOR_EACH_TAIL_INTERNAL with third argument true which may quit.  The
> earlier mentioned "And how running from a timer could be involved?"
> should explain what happens then.

Well, after gathering all the advice in this thread, I've carefully
re-checked the issue, and I must take back my original claim that timer
is somehow involved. I was able to reproduce the issue by calling
desktop-clear directly without any timer.

I was confused because second call to desktop-clear succeeds, and I came
to wrong conclusions out of pure coincidence.

Overall, buffer-list doesn't seem to be guilty in this particular case.
It's desktop-clear that is buggy. I just checked that it indeed chokes
every time there is an indirect buffer in the buffer list after its
base buffer.

Thanks,
-- Sergey Organov



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-24 20:27       ` Sergey Organov
@ 2021-05-25  6:49         ` martin rudalics
  0 siblings, 0 replies; 31+ messages in thread
From: martin rudalics @ 2021-05-25  6:49 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Stefan Monnier, emacs-devel

 > Overall, buffer-list doesn't seem to be guilty in this particular case.
 > It's desktop-clear that is buggy. I just checked that it indeed chokes
 > every time there is an indirect buffer in the buffer list after its
 > base buffer.

Well, then ... `kill-buffer' kills all indirect buffers whose base buffer
it kills.

martin



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: Should (buffer-list) ever return killed buffers?
  2021-05-23 23:41 ` Stefan Monnier
  2021-05-23 23:58   ` Sergey Organov
  2021-05-24 13:41   ` [PATCH] " Sergey Organov
@ 2021-07-22 11:27   ` Sergey Organov
  2 siblings, 0 replies; 31+ messages in thread
From: Sergey Organov @ 2021-07-22 11:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Somehow it happens when `desktop-clear` is called from a timer and there
>> is *Info* buffer out there, and apparently the one that is killed is
>>
>> "#<buffer  *info tag table*>"
>>
>> I'd like to know if this is a bug in `destop-clear`, `buffer-list`,
>> info, or elsewhere?
>
> I strongly suspect that the problem goes as follows:
> - buffer-list returns a list of buffers that are all live (i.e. no bug
>   there).
> - while processing that list, some of its buffers die.
> So I think the bug is in `desktop-clear` which should skip buffers that
> have died between the call to `buffer-list` and the moment we get to
> process them.
>
>
>         Stefan "who hasn't looked at the code"

Thanks, this is now:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=49692

-- 
Sergey Organov



^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2021-07-22 11:27 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-23 23:14 Should (buffer-list) ever return killed buffers? Sergey Organov
2021-05-23 23:41 ` Stefan Monnier
2021-05-23 23:58   ` Sergey Organov
2021-05-24  3:32     ` Stefan Monnier
2021-05-24 14:15     ` martin rudalics
2021-05-24 14:23       ` Sergey Organov
2021-05-24 16:31       ` Stefan Monnier
2021-05-24 17:05         ` martin rudalics
2021-05-24 19:55           ` Stefan Monnier
2021-05-24 20:27       ` Sergey Organov
2021-05-25  6:49         ` martin rudalics
2021-05-24 13:41   ` [PATCH] " Sergey Organov
2021-05-24 14:04     ` Tassilo Horn
2021-05-24 14:25       ` Sergey Organov
2021-05-24 14:27     ` Eli Zaretskii
2021-05-24 14:50       ` martin rudalics
2021-05-24 15:05         ` Eli Zaretskii
2021-05-24 15:32           ` Sergey Organov
2021-05-24 16:07             ` [PATCH] " Philipp
2021-05-24 18:11               ` Sergey Organov
2021-05-24 18:31               ` Sergey Organov
2021-05-24 16:25             ` [PATCH] " Eli Zaretskii
2021-05-24 18:09               ` Sergey Organov
2021-05-24 16:04           ` martin rudalics
2021-05-24 16:30             ` Eli Zaretskii
2021-05-24 19:01               ` Stefan Monnier
2021-05-24 15:14         ` Sergey Organov
2021-07-22 11:27   ` Sergey Organov
2021-05-23 23:42 ` Clément Pit-Claudel
2021-05-23 23:55   ` Sergey Organov
2021-05-23 23:56     ` Clément Pit-Claudel

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).