bug#54028: Window is not a valid window

bug#54028: Window is not a valid window

[Juri Linkov]

I don't know if the author of
https://www.reddit.com/r/emacs/comments/sthusa/window_xxx_is_not_a_valid_window_on_switching_tabs/
will report the bug here, but since it's easily reproducible,
here is the short test case, just evaluate this:

(window-state-put
 '(((min-height . 8)
    (min-width . 20)
    (min-height-ignore . 4)
    (min-width-ignore . 12)
    (min-height-safe . 2)
    (min-width-safe . 4)
    (min-pixel-height . 168)
    (min-pixel-width . 200)
    (min-pixel-height-ignore . 84)
    (min-pixel-width-ignore . 120)
    (min-pixel-height-safe . 42)
    (min-pixel-width-safe . 40))
   vc
   (pixel-width . 2048)
   (pixel-height . 1008)
   (total-width . 205)
   (total-height . 48)
   (normal-height . 1.0)
   (normal-width . 1.0)
   (combination-limit)
   (hc
    (pixel-width . 2048)
    (pixel-height . 798)
    (total-width . 205)
    (total-height . 38)
    (normal-height . 0.7916666666666667)
    (normal-width . 1.0)
    (combination-limit)
    (leaf
     (pixel-width . 1028)
     (pixel-height . 798)
     (total-width . 103)
     (total-height . 38)
     (normal-height . 1.0)
     (normal-width . 0.5)
     (buffer "test.py"
	     (selected . t)
	     (hscroll . 0)
	     (fringes 8 8 nil nil)
	     (margins nil)
	     (scroll-bars nil 2 t nil 0 t nil)
	     (vscroll . 0)
	     (dedicated)
	     (point . 72)
	     (start . 1))
     (prev-buffers
      ("cards.js" 367 1480)
      ("__init__.pyi" 21558 22551)
      ("*helpful variable: completion-at-point-functions*" 244 1615)
      ("DHU236/Lectures.org" 93135 95371)
      ("DHU236/Assignments.org" 62643 63879)
      ("DHU236/Pages.org" 591 2208)
      ("org-lms.org" 214188 214910)
      ("emacs-init.org" 127438 130382)
      ("corfu.el" 3265 4543)
      ("DHU236/Announcements.org" 12275 14041)))
    (leaf
     (last . t)
     (pixel-width . 1020)
     (pixel-height . 798)
     (total-width . 102)
     (total-height . 38)
     (normal-height . 1.0)
     (normal-width . 0.5)
     (buffer "*tree-sitter-tree: cards.js*"
	     (selected)
	     (hscroll . 0)
	     (fringes 8 8 nil nil)
	     (margins nil)
	     (scroll-bars nil 2 t nil 0 t nil)
	     (vscroll . 0)
	     (dedicated)
	     (point . 1)
	     (start . 1))))
   (leaf
    (last . t)
    (pixel-width . 2048)
    (pixel-height . 210)
    (total-width . 205)
    (total-height . 10)
    (normal-height . 0.20833333333333331)
    (normal-width . 1.0)
    (parameters
     (window-side . bottom)
     (window-slot . 0))
    (buffer "*tree-sitter-query-builder*"
	    (selected)
	    (hscroll . 0)
	    (fringes 8 8 nil nil)
	    (margins nil)
	    (scroll-bars nil 2 t nil 0 t nil)
	    (vscroll . 0)
	    (dedicated . side)
	    (point . 1)
	    (start . 1))))
 nil 'safe)

It fails with:

  Debugger entered--Lisp error: (error "#<window 9> is not a valid window")
    error("%s is not a valid window" #<window 9>)
    window-normalize-window(#<window 9>)
    window-deletable-p(#<window 9>)
    window-state-put

bug#54028: Window is not a valid window

[martin rudalics]

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

 > I don't know if the author of
 > https://www.reddit.com/r/emacs/comments/sthusa/window_xxx_is_not_a_valid_window_on_switching_tabs/
 > will report the bug here,

Please encourage the author again to report such bugs here.

 > but since it's easily reproducible,
 > here is the short test case, just evaluate this:
 >
 > (window-state-put
 > ...
 >   nil 'safe)
 >
 > It fails with:
 >
 >    Debugger entered--Lisp error: (error "#<window 9> is not a valid window")
 >      error("%s is not a valid window" #<window 9>)
 >      window-normalize-window(#<window 9>)
 >      window-deletable-p(#<window 9>)
 >      window-state-put

I think the attached fixes it, please check.  I have no idea whether the
state is restored as intended, though.

Thanks, martin

[-- Attachment #2: window-state-put.diff --]
[-- Type: text/x-patch, Size: 512 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 582600e1c6..38b58712b4 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6394,7 +6394,8 @@ window-state-put
 	(window--state-put-2 ignore pixelwise))
       (while window-state-put-stale-windows
 	(let ((window (pop window-state-put-stale-windows)))
-	  (when (eq (window-deletable-p window) t)
+	  (when (and (window-valid-p window)
+                     (eq (window-deletable-p window) t))
 	    (delete-window window))))
       (window--check frame))))
 

bug#54028: Window is not a valid window

[Juri Linkov]

>> I don't know if the author of
>> https://www.reddit.com/r/emacs/comments/sthusa/window_xxx_is_not_a_valid_window_on_switching_tabs/
>> will report the bug here,
>
> Please encourage the author again to report such bugs here.

Maybe they hesitate to use e-mail?

>> but since it's easily reproducible,
>> here is the short test case, just evaluate this:
>>
>> (window-state-put
>> ...
>>   nil 'safe)
>>
>> It fails with:
>>
>>    Debugger entered--Lisp error: (error "#<window 9> is not a valid window")
>>      error("%s is not a valid window" #<window 9>)
>>      window-normalize-window(#<window 9>)
>>      window-deletable-p(#<window 9>)
>>      window-state-put
>
> I think the attached fixes it, please check.

Thanks, it doesn't fail anymore, but wasn't the problem
somewhere in the invalid window state?

> I have no idea whether the state is restored as intended, though.

Is it possible to find what window was invalid in this window state?

bug#54028: Window is not a valid window

[martin rudalics]

 > Thanks, it doesn't fail anymore, but wasn't the problem
 > somewhere in the invalid window state?

No.  The sequence of events leading to the bug is the following:
'window-state-put' has

       ;; Work on the windows of a temporary buffer to make sure that
       ;; splitting proceeds regardless of any buffer local values of
       ;; `window-size-fixed'.  Release that buffer after the buffers of
       ;; all live windows have been set by `window--state-put-2'.
       (with-temp-buffer
	(set-window-buffer window (current-buffer))
	(window--state-put-1 state window nil totals pixelwise)
	(window--state-put-2 ignore pixelwise))

When this calls 'window--state-put-2', the window's buffer is *temp*.
'window--state-put-2' has:

	    ;; We don't want to raise an error in case the buffer does
	    ;; not exist anymore, so we switch to a previous one and
	    ;; save the window with the intention of deleting it later
	    ;; if possible.
	    (switch-to-prev-buffer window)
	    (push window window-state-put-stale-windows)))))))

Here, since window has no previous buffer, *temp* remains that window's
buffer.  Now *temp* gets killed when the 'with-temp-buffer' form exits
and, since its window is deletable, that window simply gets deleted via
‘kill-buffer’.  Back in 'window-state-put' we do

       (while window-state-put-stale-windows
         (let ((window (pop window-state-put-stale-windows)))
           (when (eq (window-deletable-p window) t)
	    (delete-window window))))

and here the window is no more valid because it has been deleted in
the meantime.

So AFAICT we start with a valid window state 'window-state-put' can't
handle.  I have no idea why this didn't hit us before.  Maybe, because
such windows usually have a previous buffer 'switch-to-prev-buffer' can
switch to.

 >> I have no idea whether the state is restored as intended, though.
 >
 > Is it possible to find what window was invalid in this window state?

There is none as I tried to explain above.  The question is rather
whether the restored state resembles the saved state satisfactorily.
But that only the OP can tell.

martin

bug#54028: Window is not a valid window

[martin rudalics]

Eli, is the below OK to install on the release branch?  It's a trivial
safety check that would fix this bug.

Thanks, martin


--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6394,7 +6394,8 @@ window-state-put
  	(window--state-put-2 ignore pixelwise))
        (while window-state-put-stale-windows
  	(let ((window (pop window-state-put-stale-windows)))
-	  (when (eq (window-deletable-p window) t)
+	  (when (and (window-valid-p window)
+                     (eq (window-deletable-p window) t))
  	    (delete-window window))))
        (window--check frame))))

bug#54028: Window is not a valid window

[Eli Zaretskii]

> Date: Mon, 21 Feb 2022 10:07:59 +0100
> From: martin rudalics <rudalics@gmx.at>
> Cc: "eliz@gnu.org" <eliz@gnu.org>
> 
> Eli, is the below OK to install on the release branch?  It's a trivial
> safety check that would fix this bug.

Yes, thanks.

bug#54028: Window is not a valid window

[martin rudalics]

close 54028 28.1
quit

 >> Eli, is the below OK to install on the release branch?  It's a trivial
 >> safety check that would fix this bug.
 >
 > Yes, thanks.

Fixed with commit 68d134cf0f784601e50cefc4056eb0ace510a982 on the
release branch.  Closing this bug.

Thanks, martin