From d38ba7e2b8597de2d89a6ccf21aedd47effa4631 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 8c0fb9a230..aa1a8b2df9 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) { @@ -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 index 7b5c1269c9..1fec341045 100644 --- 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 index 59ed629045..442f6e86e7 100644 --- 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()