fix(window): restore b_nwindows if win_close_othertab keeps window

seandewar · zeertzjq · commit d38ba7e2b859 · 2026-01-05T11:33:26.000+08:00
Problem: can't accurately know if close_buffer directly (e.g: not via autocmds)
decremented b_nwindows. This can cause crashes if win_close_othertab decides to
keep the window after calling close_buffer (if it did not free the buffer), as
b_nwindows may remain out-of-sync.

Solution: change the return value of close_buffer to accurately depict whether
it decremented b_nwindows. Check it in win_close_othertab to avoid a crash.

Similar issues may exist in other places that call close_buffer, but I've not
addressed those here (not to mention only one other place even checks its return
value...)
diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
@@ -500,7 +500,7 @@ static bool can_unload_buffer(buf_T *buf)
 ///               close all other windows.
 /// @param ignore_abort
 ///               If true, don't abort even when aborting() returns true.
-/// @return  true when we got to the end and b_nwindows was decremented.
+/// @return  true if b_nwindows was decremented directly by this call (e.g: not via autocmds).
 bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool ignore_abort)
 {
   bool unload_buf = (action != 0);
@@ -625,7 +625,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i
   // Return when a window is displaying the buffer or when it's not
   // unloaded.
   if (buf->b_nwindows > 0 || !unload_buf) {
-    return false;
+    return true;
   }
 
   if (buf->terminal) {
@@ -702,7 +702,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i
     }
     // Do not wipe out the buffer if it is used in a window.
     if (buf->b_nwindows > 0) {
-      return false;
+      return true;
     }
     FOR_ALL_TAB_WINDOWS(tp, wp) {
       mark_forget_file(wp, buf->b_fnum);
diff --git a/src/nvim/window.c b/src/nvim/window.c
@@ -2984,6 +2984,7 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force)
   FUNC_ATTR_NONNULL_ALL
 {
   assert(tp != curtab);
+  bool did_decrement = false;
 
   // Get here with win->w_buffer == NULL when win_close() detects the tab page
   // changed.
@@ -3023,9 +3024,12 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force)
     }
   }
 
+  bufref_T bufref;
+  set_bufref(&bufref, win->w_buffer);
+
   if (win->w_buffer != NULL) {
     // Close the link to the buffer.
-    close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true);
+    did_decrement = close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true);
   }
 
   // Careful: Autocommands may have closed the tab page or made it the
@@ -3089,11 +3093,17 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force)
   return true;
 
 leave_open:
-  // If the buffer was removed from the window we have to give it any buffer.
-  if (win_valid_any_tab(win) && win->w_buffer == NULL) {
-    win->w_buffer = firstbuf;
-    firstbuf->b_nwindows++;
-    win_init_empty(win);
+  if (win_valid_any_tab(win)) {
+    if (win->w_buffer == NULL) {
+      // If the buffer was removed from the window we have to give it any buffer.
+      win->w_buffer = firstbuf;
+      firstbuf->b_nwindows++;
+      win_init_empty(win);
+    } else if (did_decrement && win->w_buffer == bufref.br_buf && bufref_valid(&bufref)) {
+      // close_buffer decremented the window count, but we're keeping the window.
+      // As the window is still viewing the buffer, increment the count.
+      win->w_buffer->b_nwindows++;
+    }
   }
   return false;
 }
diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua
@@ -1986,6 +1986,23 @@ describe('API/win', function()
         pcall_err(command, 'call nvim_win_close(g:win, 0)')
       )
       eq(true, eval 'nvim_tabpage_is_valid(g:tp)')
+
+      exec([[
+        tabnew
+        let g:tp = nvim_get_current_tabpage()
+        let g:win = win_getid()
+        let g:buf = bufnr()
+        tabprevious
+        let s:buf2 = nvim_create_buf(0, 0)
+        call setbufvar(s:buf2, '&modified', 1)
+        call setbufvar(s:buf2, '&bufhidden', 'wipe')
+        autocmd! WinClosed * ++once call nvim_open_win(s:buf2, 0, #{win: g:win, relative: 'win', width: 5, height: 5, row: 5, col: 5})
+      ]])
+      matches(
+        'E5601: Cannot close window, only floating window would remain$',
+        pcall_err(command, 'call nvim_buf_delete(g:buf, #{force: 1})')
+      )
+      eq(true, eval 'nvim_tabpage_is_valid(g:tp)')
     end)
 
     it('respects requested size for large splits', function()

Original file line number	Diff line number	Diff line change
`@@ -500,7 +500,7 @@ static bool can_unload_buffer(buf_T *buf)`
`500`	`500`	`/// close all other windows.`
`501`	`501`	`/// @param ignore_abort`
`502`	`502`	`/// If true, don't abort even when aborting() returns true.`
`503`		`-/// @return true when we got to the end and b_nwindows was decremented.`
	`503`	`+/// @return true if b_nwindows was decremented directly by this call (e.g: not via autocmds).`
`504`	`504`	`bool close_buffer(win_T win, buf_T buf, int action, bool abort_if_last, bool ignore_abort)`
`505`	`505`	`{`
`506`	`506`	`bool unload_buf = (action != 0);`
`@@ -625,7 +625,7 @@ bool close_buffer(win_T win, buf_T buf, int action, bool abort_if_last, bool i`
`625`	`625`	`// Return when a window is displaying the buffer or when it's not`
`626`	`626`	`// unloaded.`
`627`	`627`	`if (buf->b_nwindows > 0 \|\| !unload_buf) {`
`628`		`- return false;`
	`628`	`+ return true;`
`629`	`629`	`}`
`630`	`630`
`631`	`631`	`if (buf->terminal) {`
`@@ -702,7 +702,7 @@ bool close_buffer(win_T win, buf_T buf, int action, bool abort_if_last, bool i`
`702`	`702`	`}`
`703`	`703`	`// Do not wipe out the buffer if it is used in a window.`
`704`	`704`	`if (buf->b_nwindows > 0) {`
`705`		`- return false;`
	`705`	`+ return true;`
`706`	`706`	`}`
`707`	`707`	`FOR_ALL_TAB_WINDOWS(tp, wp) {`
`708`	`708`	`mark_forget_file(wp, buf->b_fnum);`