From 9813c00dd376c00c382aaf0a8f0a6d0f45ba2a5d Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed, 16 Jul 2025 20:44:18 +0100 Subject: [PATCH] fix(window): restore b_nwindows if win_close_othertab keeps window 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...) --- src/nvim/buffer.c | 6 +++--- src/nvim/window.c | 22 ++++++++++++++++------ test/functional/api/window_spec.lua | 17 +++++++++++++++++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index 3f88064ca8..4ab78b9f8a 100644 --- 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) { @@ -699,7 +699,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i if (wipe_buf) { // 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 index 5d3774a40e..5788b97ca7 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2979,6 +2979,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. @@ -3018,9 +3019,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 @@ -3084,11 +3088,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 index e4565119fd..4ce3444302 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2209,6 +2209,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) end)