bug#11665: kill-buffer gives an error on killing dead buffers

bug#11665: kill-buffer gives an error on killing dead buffers

[Vitalie Spinu]

Hi, 

This is a strange behavior which was not happening in previous versions
of emacs. Though quite an unusual use, I decided to report it as it
might be a symptom for a more serious problem.

Create a buffer "test.tmp" and create an indirect buffer:
   
   (with-current-buffer (make-indirect-buffer "test.tmp" "test1")
     	    (add-hook 'kill-buffer-hook
   		      '(lambda ()
                            (setq kill-buffer-hook nil)
                            (kill-buffer (buffer-base-buffer)))
   		      t t))

(The above triggers the killing of the base buffer when an indirect
buffer is killed)

Now, kill the base buffer "test.tmp", which raises:

  signal(error ("Buffer #<killed buffer> is not a live buffer"))
  error("Buffer %s is not a live buffer" #<killed buffer>)
  window-normalize-buffer(#<killed buffer>)
  replace-buffer-in-windows(#<killed buffer>)
  kill-buffer(#<killed buffer>)
  (if (and (boundp (quote sub-kill-buffer-and-its-windows)) sub-kill-buffer-and-its-windows (fboundp (quote kill-buffer-and-its-windows))) (kill-buffer-and-its-windows (current-buffer)) (kill-buffer (current-buffer)))


This doesn't occur when killing an indirect buffer. 

Vitalie.


In GNU Emacs 24.1.50.1 (i686-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2012-06-08 on lakoocha, modified by Debian
 (emacs-snapshot package, version 2:20120608-1~ppa1~precise1)
Windowing system distributor `The X.Org Foundation', version 11.0.11103000
Configured using:
 `configure '--build' 'i686-linux-gnu' '--host' 'i686-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.1.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.1.50/site-lisp:/usr/share/emacs/site-lisp'
 '--without-compress-info' '--with-crt-dir=/usr/lib/i386-linux-gnu/'
 '--with-x=yes' '--with-x-toolkit=gtk3' '--with-imagemagick=yes'
 'build_alias=i686-linux-gnu' 'host_alias=i686-linux-gnu'
 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2' 'LDFLAGS=-g
 -Wl,--as-needed -znocombreloc' 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  TeX-PDF-mode: t
  highlight-parentheses-mode: t
  global-auto-complete-mode: t
  auto-complete-mode: t
  diff-auto-refine-mode: t
  global-auto-revert-mode: t
  keyfreq-autosave-mode: t
  keyfreq-mode: t
  sr-popviewer-mode: t
  shell-dirtrack-mode: t
  eldoc-mode: t
  display-time-mode: t
  ido-everywhere: t
  show-paren-mode: t
  savehist-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  hs-minor-mode: t

   

bug#11665: kill-buffer gives an error on killing dead buffers

[martin rudalics]

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

 > Create a buffer "test.tmp" and create an indirect buffer:
 >
 >    (with-current-buffer (make-indirect-buffer "test.tmp" "test1")
 >      	    (add-hook 'kill-buffer-hook
 >    		      '(lambda ()
 >                             (setq kill-buffer-hook nil)
 >                             (kill-buffer (buffer-base-buffer)))
 >    		      t t))
 >
 > (The above triggers the killing of the base buffer when an indirect
 > buffer is killed)
 >
 > Now, kill the base buffer "test.tmp", which raises:
 >
 >   signal(error ("Buffer #<killed buffer> is not a live buffer"))
 >   error("Buffer %s is not a live buffer" #<killed buffer>)
 >   window-normalize-buffer(#<killed buffer>)
 >   replace-buffer-in-windows(#<killed buffer>)
 >   kill-buffer(#<killed buffer>)
 >   (if (and (boundp (quote sub-kill-buffer-and-its-windows)) sub-kill-buffer-and-its-windows (fboundp (quote kill-buffer-and-its-windows))) (kill-buffer-and-its-windows (current-buffer)) (kill-buffer (current-buffer)))
 >
 >
 > This doesn't occur when killing an indirect buffer.

I don't have `sub-kill-buffer-and-its-windows' so I can't repeat this
easily.  Anyway, here's what I suppose to happen:

Killing "test.tmp" implicitly calls `kill-buffer' on "test1" because
killing a base buffer kills all its indirect buffers.

    Killing "test1" calls `kill-buffer' on "test.tmp" because that's on
    the hook.

       Killing "test.tmp" calls `kill-buffer' on "test1" again.

          Killing "test1" now succeeds and returns.
	
       Killing "test.tmp" now succeeds.

   Killing "test1" now tries to continue with `replace-buffer-in-windows'
   but this fails because the buffer is no more live.

I also suppose that the

 >   kill-buffer(#<killed buffer>)

is a red herring in the sense that `kill-buffer' was actually invoked
with a live buffer but when the trace was printed the buffer was already
dead while `replace-buffer-in-windows' was really called with a dead
buffer as argument.

Basically, I could exit `replace-buffer-in-windows' when the argument
buffer is not live but that's not nice.  So maybe the attached patch is
better.  Can you try it?

martin

[-- Attachment #2: buffer.c.diff --]
[-- Type: text/plain, Size: 514 bytes --]

*** src/buffer.c	2012-05-29 16:13:38 +0000
--- src/buffer.c	2012-06-10 12:52:57 +0000
***************
*** 1526,1531 ****
--- 1526,1535 ----
        UNGCPRO;
      }

+   /* If killing the indirect buffers has killed our buffer, return.  */
+   if (NILP (BVAR (b, name)))
+     return Qnil;
+ 
    /* Run replace_buffer_in_windows before making another buffer current
       since set-window-buffer-start-and-point will refuse to make another
       buffer current if the selected window does not show the current

bug#11665: kill-buffer gives an error on killing dead buffers

[martin rudalics]

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

There seem more occasions where this problem could strike.  Also, we
should return t when we find out that the buffer name has been reset to
nil under our feet.  Patch attached, comments welcome.

martin

[-- Attachment #2: buffer.c.diff --]
[-- Type: text/plain, Size: 3296 bytes --]

*** src/buffer.c	2012-05-29 16:13:38 +0000
--- src/buffer.c	2012-06-10 17:33:15 +0000
***************
*** 1493,1498 ****
--- 1493,1502 ----
      unbind_to (count, Qnil);
    }
  
+   /* If the hooks have killed the buffer, exit now.  */
+   if (NILP (BVAR (b, name)))
+     return Qt;
+ 
    /* We have no more questions to ask.  Verify that it is valid
       to kill the buffer.  This must be done after the questions
       since anything can happen within do_yes_or_no_p.  */
***************
*** 1501,1509 ****
    if (EQ (buffer, XWINDOW (minibuf_window)->buffer))
      return Qnil;
  
-   if (NILP (BVAR (b, name)))
-     return Qnil;
- 
    /* When we kill a base buffer, kill all its indirect buffers.
       We do it at this stage so nothing terrible happens if they
       ask questions or their hooks get errors.  */
--- 1505,1510 ----
***************
*** 1526,1540 ****
        UNGCPRO;
      }
  
    /* Run replace_buffer_in_windows before making another buffer current
       since set-window-buffer-start-and-point will refuse to make another
       buffer current if the selected window does not show the current
       buffer.  (Bug#10114) */
    replace_buffer_in_windows (buffer);
  
!      /* Make this buffer not be current.
!      In the process, notice if this is the sole visible buffer
!      and give up if so.  */
    if (b == current_buffer)
      {
        tem = Fother_buffer (buffer, Qnil, Qnil);
--- 1527,1549 ----
        UNGCPRO;
      }
  
+   /* If killing the indirect buffers has killed our buffer, return.  */
+   if (NILP (BVAR (b, name)))
+     return Qt;
+ 
    /* Run replace_buffer_in_windows before making another buffer current
       since set-window-buffer-start-and-point will refuse to make another
       buffer current if the selected window does not show the current
       buffer.  (Bug#10114) */
    replace_buffer_in_windows (buffer);
  
!   /* If replacing the buffer in windows has killed our buffer,
!      return.  */
!   if (NILP (BVAR (b, name)))
!     return Qt;
! 
!   /* Make this buffer not be current.  In the process, notice if this is
!      the sole visible buffer and give up if so.  */
    if (b == current_buffer)
      {
        tem = Fother_buffer (buffer, Qnil, Qnil);
***************
*** 1564,1574 ****
    kill_buffer_processes (buffer);
    UNGCPRO;
  
!   /* Killing buffer processes may run sentinels which may
!      have called kill-buffer.  */
! 
    if (NILP (BVAR (b, name)))
!     return Qnil;
  
    /* These may run Lisp code and into infinite loops (if someone
       insisted on circular lists) so allow quitting here.  */
--- 1573,1582 ----
    kill_buffer_processes (buffer);
    UNGCPRO;
  
!   /* Killing buffer processes may run sentinels which may have called
!      kill-buffer.  */
    if (NILP (BVAR (b, name)))
!     return Qt;
  
    /* These may run Lisp code and into infinite loops (if someone
       insisted on circular lists) so allow quitting here.  */
***************
*** 1599,1604 ****
--- 1607,1616 ----
  	internal_delete_file (BVAR (b, auto_save_file_name));
      }
  
+   /* Deleting a file could have killed our buffer.  */
+   if (NILP (BVAR (b, name)))
+     return Qt;
+ 
    if (b->base_buffer)
      {
        /* Unchain all markers that belong to this indirect buffer.

bug#11665: kill-buffer gives an error on killing dead buffers

[martin rudalics]

> There seem more occasions where this problem could strike.  Also, we
> should return t when we find out that the buffer name has been reset to
> nil under our feet.  Patch attached, comments welcome.

Changes applied to trunk, bug closed.

martin