From 88619e1aafb106ff98f5d42fb8b842139d99f53b Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 15 Jul 2025 23:16:40 +0100 Subject: [PATCH 01/10] fix(window): handle closing the only non-float in other tabpage Problem: No check for closing the only non-floating window in a non-current tabpage that contains floats. This can lead to a tabpage that contains only floats, causing crashes. Solution: Copy the relevant check from win_close to win_close_othertab. Fix some uncovered issues. Closes #34943 Fixes #31236 Co-authored-by: glepnir --- src/nvim/api/window.c | 2 +- src/nvim/ex_docmd.c | 2 +- src/nvim/window.c | 106 +++++++++++++++--------- test/functional/api/window_spec.lua | 57 +++++++++++++ test/functional/editor/tabpage_spec.lua | 9 ++ 5 files changed, 134 insertions(+), 42 deletions(-) diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c index cde629dab3..2bdb11d285 100644 --- a/src/nvim/api/window.c +++ b/src/nvim/api/window.c @@ -374,7 +374,7 @@ void nvim_win_hide(Window window, Error *err) } else if (tabpage == curtab) { win_close(win, false, false); } else { - win_close_othertab(win, false, tabpage); + win_close_othertab(win, false, tabpage, false); } }); } diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index 0a52b7e909..a3abe75150 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -4915,7 +4915,7 @@ void ex_win_close(int forceit, win_T *win, tabpage_T *tp) if (tp == NULL) { win_close(win, !need_hide && !buf_hide(buf), forceit); } else { - win_close_othertab(win, !need_hide && !buf_hide(buf), tp); + win_close_othertab(win, !need_hide && !buf_hide(buf), tp, forceit); } } diff --git a/src/nvim/window.c b/src/nvim/window.c index 7cfe436337..5a87f113da 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2520,7 +2520,10 @@ void close_windows(buf_T *buf, bool keep_curwin) for (win_T *wp = tp->tp_lastwin; wp != NULL; wp = wp->w_prev) { if (wp->w_buffer == buf && !(win_locked(wp) || wp->w_buffer->b_locked > 0)) { - win_close_othertab(wp, false, tp); + if (!win_close_othertab(wp, false, tp, false)) { + // If closing the window fails give up, to avoid looping forever. + break; + } // Start all over, the tab page may be closed and // autocommands may change the window layout. @@ -2550,14 +2553,15 @@ bool one_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT return firstwin == win && (win->w_next == NULL || win->w_next->w_floating); } -/// Check if floating windows in the current tab can be closed. +/// Check if floating windows in tabpage `tp` can be closed. /// Do not call this when the autocommand window is in use! /// +/// @param tp tabpage to check. Must be NULL for the current tabpage. /// @return true if all floating windows can be closed -static bool can_close_floating_windows(void) +static bool can_close_floating_windows(tabpage_T *tp) { - assert(!is_aucmd_win(lastwin)); - for (win_T *wp = lastwin; wp->w_floating; wp = wp->w_prev) { + assert(tp != curtab && (tp || !is_aucmd_win(lastwin))); + for (win_T *wp = tp ? tp->tp_lastwin : lastwin; wp->w_floating; wp = wp->w_prev) { buf_T *buf = wp->w_buffer; int need_hide = (bufIsChanged(buf) && buf->b_nwindows <= 1); @@ -2618,10 +2622,10 @@ static bool close_last_window_tabpage(win_T *win, bool free_buf, tabpage_T *prev // case they have already been triggered. goto_tabpage_tp(alt_tabpage(), false, win->w_buffer != NULL); - // Safety check: Autocommands may have closed the window when jumping - // to the other tab page. - if (valid_tabpage(prev_curtab) && prev_curtab->tp_firstwin == win) { - win_close_othertab(win, free_buf, prev_curtab); + // Safety check: Autocommands may have switched back to the old tab page + // or closed the window when jumping to the other tab page. + if (curtab != prev_curtab && valid_tabpage(prev_curtab) && prev_curtab->tp_firstwin == win) { + win_close_othertab(win, free_buf, prev_curtab, false); } entering_window(curwin); @@ -2702,7 +2706,7 @@ int win_close(win_T *win, bool free_buf, bool force) emsg(_("E814: Cannot close window, only autocmd window would remain")); return FAIL; } - if (force || can_close_floating_windows()) { + if (force || can_close_floating_windows(NULL)) { // close the last window until the there are no floating windows while (lastwin->w_floating) { // `force` flag isn't actually used when closing a floating window. @@ -2808,7 +2812,7 @@ int win_close(win_T *win, bool free_buf, bool force) if (curtab != prev_curtab && win_valid_any_tab(win) && win->w_buffer == NULL) { // Need to close the window anyway, since the buffer is NULL. - win_close_othertab(win, false, prev_curtab); + win_close_othertab(win, false, prev_curtab, force); return FAIL; } @@ -2974,14 +2978,38 @@ static void do_autocmd_winclosed(win_T *win) // thus "tp" may become invalid! // Caller must check if buffer is hidden and whether the tabline needs to be // updated. -void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) +// @return false when the window was not closed as a direct result of this call +// (e.g: not via autocmds). +bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) FUNC_ATTR_NONNULL_ALL { + assert(tp != curtab); + // Get here with win->w_buffer == NULL when win_close() detects the tab page // changed. if (win_locked(win) || (win->w_buffer != NULL && win->w_buffer->b_locked > 0)) { - return; // window is already being closed + return false; // window is already being closed + } + + // Check if closing this window would leave only floating windows. + if (tp->tp_firstwin == win && win->w_next && win->w_next->w_floating) { + if (force || can_close_floating_windows(tp)) { + // close the last window until the there are no floating windows + while (tp->tp_lastwin->w_floating) { + // `force` flag isn't actually used when closing a floating window. + if (!win_close_othertab(tp->tp_lastwin, free_buf, tp, true)) { + // If closing the window fails give up, to avoid looping forever. + goto leave_open; + } + } + if (!win_valid_any_tab(win)) { + return false; // window already closed by autocommands + } + } else { + emsg(e_floatonly); + goto leave_open; + } } // Fire WinClosed just before starting to free window-related resources. @@ -2991,7 +3019,7 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) do_autocmd_winclosed(win); // autocmd may have freed the window already. if (!win_valid_any_tab(win)) { - return; + return false; } } @@ -3000,34 +3028,21 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true); } - tabpage_T *ptp = NULL; - // Careful: Autocommands may have closed the tab page or made it the // current tab page. - for (ptp = first_tabpage; ptp != NULL && ptp != tp; ptp = ptp->tp_next) {} - if (ptp == NULL || tp == curtab) { - // 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); - } - return; + if (!valid_tabpage(tp) || tp == curtab) { + goto leave_open; } - - // Autocommands may have closed the window already. - { - bool found_window = false; - FOR_ALL_WINDOWS_IN_TAB(wp, tp) { - if (wp == win) { - found_window = true; - break; - } - } - if (!found_window) { - return; - } + // Autocommands may have closed the window already, or nvim_win_set_config + // moved it to a different tab page. + if (!tabpage_win_valid(tp, win)) { + goto leave_open; + } + // Autocommands may again cause closing this window to leave only floats. + // Check again; we'll not bother closing floating windows this time. + if (tp->tp_firstwin == win && win->w_next && win->w_next->w_floating) { + emsg(e_floatonly); + goto leave_open; } bool free_tp = false; @@ -3044,13 +3059,14 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) if (tp == first_tabpage) { first_tabpage = tp->tp_next; } else { + tabpage_T *ptp; for (ptp = first_tabpage; ptp != NULL && ptp->tp_next != tp; ptp = ptp->tp_next) { // loop } if (ptp == NULL) { internal_error("win_close_othertab()"); - return; + return false; } ptp->tp_next = tp->tp_next; } @@ -3072,6 +3088,16 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) if (free_tp) { free_tabpage(tp); } + 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); + } + return false; } /// Free the memory used for a window. diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index f1579f5345..59ed629045 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -1931,6 +1931,63 @@ describe('API/win', function() ) end) + it('no crash when closing the only non-float in other tabpage #31236', function() + local tp = api.nvim_get_current_tabpage() + local split_win = api.nvim_get_current_win() + local float_win = api.nvim_open_win( + 0, + false, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + command('tabnew') + + api.nvim_win_close(split_win, false) + eq(false, api.nvim_win_is_valid(split_win)) + eq(false, api.nvim_win_is_valid(float_win)) + eq(false, api.nvim_tabpage_is_valid(tp)) + + tp = api.nvim_get_current_tabpage() + split_win = api.nvim_get_current_win() + local float_buf = api.nvim_create_buf(true, false) + float_win = api.nvim_open_win( + float_buf, + false, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + -- Set these options to prevent the float from being automatically closed. + api.nvim_set_option_value('modified', true, { buf = float_buf }) + api.nvim_set_option_value('bufhidden', 'wipe', { buf = float_buf }) + command('tabnew') + + matches( + 'E5601: Cannot close window, only floating window would remain$', + pcall_err(api.nvim_win_close, split_win, false) + ) + eq(true, api.nvim_win_is_valid(split_win)) + eq(true, api.nvim_win_is_valid(float_win)) + eq(true, api.nvim_tabpage_is_valid(tp)) + + api.nvim_set_current_win(float_win) + api.nvim_win_close(split_win, true) -- Force it this time. + eq(false, api.nvim_win_is_valid(split_win)) + eq(false, api.nvim_win_is_valid(float_win)) + eq(false, api.nvim_tabpage_is_valid(tp)) + + -- Ensure opening a float after the initial check (like in WinClosed) doesn't crash... + exec([[ + tabnew + let g:tp = nvim_get_current_tabpage() + let g:win = win_getid() + tabprevious + autocmd! WinClosed * ++once call nvim_open_win(0, 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_win_close(g:win, 0)') + ) + eq(true, eval 'nvim_tabpage_is_valid(g:tp)') + end) + it('respects requested size for large splits', function() command('vsplit') local win = api.nvim_open_win(0, false, { win = -1, split = 'right', width = 38 }) diff --git a/test/functional/editor/tabpage_spec.lua b/test/functional/editor/tabpage_spec.lua index e829821a64..c8be39ea6e 100644 --- a/test/functional/editor/tabpage_spec.lua +++ b/test/functional/editor/tabpage_spec.lua @@ -147,4 +147,13 @@ describe('tabpage', function() command('tabs') assert_alive() end) + + it('no crash if autocmd remains in tabpage of closing last window', function() + exec([[ + tabnew + let s:win = win_getid() + autocmd TabLeave * ++once tablast | tabonly + quit + ]]) + end) end) From 46011a4e874d47d8ca7f1e0d68fcb685944d45c3 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed, 16 Jul 2025 10:18:19 +0100 Subject: [PATCH 02/10] fix(autocmd): fire TabClosed after freeing tab page Problem: TabClosed is fired after close_buffer is called (after b_nwindows is decremented) and after the tab page is removed from the list, but before it's freed. This causes inconsistencies such as the removed tabpage having a valid handle and functions like nvim_tabpage_get_number returning nonsense. Solution: fire it after free_tabpage. Try to maintain the Nvim-specific behaviour of setting `` to the old tab page number, and the (undocumented) behaviour of setting `` to the buffer it was showing (close_buffer sets w_buffer to NULL if it was freed, so it should be OK pass it to apply_autocmds_group, similar to before). --- src/nvim/window.c | 22 +++++++--------- test/functional/autocmd/tabclose_spec.lua | 32 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/nvim/window.c b/src/nvim/window.c index 5a87f113da..7b5c1269c9 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -3045,15 +3045,11 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) goto leave_open; } - bool free_tp = false; + int free_tp_idx = 0; // When closing the last window in a tab page remove the tab page. if (tp->tp_firstwin == tp->tp_lastwin) { - char prev_idx[NUMBUFLEN]; - if (has_event(EVENT_TABCLOSED)) { - vim_snprintf(prev_idx, NUMBUFLEN, "%i", tabpage_index(tp)); - } - + free_tp_idx = tabpage_index(tp); int h = tabline_height(); if (tp == first_tabpage) { @@ -3070,23 +3066,25 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) } ptp->tp_next = tp->tp_next; } - free_tp = true; redraw_tabline = true; if (h != tabline_height()) { win_new_screen_rows(); } - - if (has_event(EVENT_TABCLOSED)) { - apply_autocmds(EVENT_TABCLOSED, prev_idx, prev_idx, false, win->w_buffer); - } } // Free the memory used for the window. + buf_T *buf = win->w_buffer; int dir; win_free_mem(win, &dir, tp); - if (free_tp) { + if (free_tp_idx > 0) { free_tabpage(tp); + + if (has_event(EVENT_TABCLOSED)) { + char prev_idx[NUMBUFLEN]; + vim_snprintf(prev_idx, NUMBUFLEN, "%i", free_tp_idx); + apply_autocmds(EVENT_TABCLOSED, prev_idx, prev_idx, false, buf); + } } return true; diff --git a/test/functional/autocmd/tabclose_spec.lua b/test/functional/autocmd/tabclose_spec.lua index fde72cf4d7..b6651b9c31 100644 --- a/test/functional/autocmd/tabclose_spec.lua +++ b/test/functional/autocmd/tabclose_spec.lua @@ -4,6 +4,8 @@ local n = require('test.functional.testnvim')() local clear, eq = n.clear, t.eq local api = n.api local command = n.command +local eval = n.eval +local exec = n.exec describe('TabClosed', function() before_each(clear) @@ -48,6 +50,36 @@ describe('TabClosed', function() eq('tabclosed:2:2:2', api.nvim_exec('bdelete Xtestfile2', true)) eq('Xtestfile1', api.nvim_eval('bufname("")')) end) + + it('triggers after tab page is properly freed', function() + exec([[ + let s:tp = nvim_get_current_tabpage() + let g:buf = bufnr() + + setlocal bufhidden=wipe + tabnew + au TabClosed * ++once let g:tp_valid = nvim_tabpage_is_valid(s:tp) + \| let g:abuf = expand('') + + call nvim_buf_delete(g:buf, #{force: 1}) + ]]) + eq(false, eval('g:tp_valid')) + eq(false, eval('nvim_buf_is_valid(g:buf)')) + eq('', eval('g:abuf')) + + exec([[ + tabnew + let g:buf = bufnr() + let s:win = win_getid() + + tabfirst + au TabClosed * ++once let g:abuf = expand('') + + call nvim_win_close(s:win, 1) + ]]) + eq(true, eval('nvim_buf_is_valid(g:buf)')) + eq(eval('g:buf'), tonumber(eval('g:abuf'))) + end) end) describe('with NR as ', function() From a3dabcfa11615466c6d14d6e17d451d6c9d8a0bd Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Sat, 19 Jul 2025 18:18:15 +0100 Subject: [PATCH 03/10] test(tabclose): remove deprecated calls, use testnvim helpers --- test/functional/autocmd/tabclose_spec.lua | 36 +++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/test/functional/autocmd/tabclose_spec.lua b/test/functional/autocmd/tabclose_spec.lua index b6651b9c31..3969f33cf1 100644 --- a/test/functional/autocmd/tabclose_spec.lua +++ b/test/functional/autocmd/tabclose_spec.lua @@ -2,10 +2,10 @@ local t = require('test.testutil') local n = require('test.functional.testnvim')() local clear, eq = n.clear, t.eq -local api = n.api local command = n.command local eval = n.eval local exec = n.exec +local exec_capture = n.exec_capture describe('TabClosed', function() before_each(clear) @@ -18,11 +18,11 @@ describe('TabClosed', function() ) repeat command('tabnew') - until api.nvim_eval('tabpagenr()') == 6 -- current tab is now 6 - eq('tabclosed:6:6:5', api.nvim_exec('tabclose', true)) -- close last 6, current tab is now 5 - eq('tabclosed:5:5:4', api.nvim_exec('close', true)) -- close last window on tab, closes tab - eq('tabclosed:2:2:3', api.nvim_exec('2tabclose', true)) -- close tab 2, current tab is now 3 - eq('tabclosed:1:1:2\ntabclosed:1:1:1', api.nvim_exec('tabonly', true)) -- close tabs 1 and 2 + until eval('tabpagenr()') == 6 -- current tab is now 6 + eq('tabclosed:6:6:5', exec_capture('tabclose')) -- close last 6, current tab is now 5 + eq('tabclosed:5:5:4', exec_capture('close')) -- close last window on tab, closes tab + eq('tabclosed:2:2:3', exec_capture('2tabclose')) -- close tab 2, current tab is now 3 + eq('tabclosed:1:1:2\ntabclosed:1:1:1', exec_capture('tabonly')) -- close tabs 1 and 2 end) it('is triggered when closing a window via bdelete from another tab', function() @@ -32,9 +32,9 @@ describe('TabClosed', function() command('1tabedit Xtestfile') command('1tabedit Xtestfile') command('normal! 1gt') - eq({ 1, 3 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) - eq('tabclosed:2:2:1\ntabclosed:2:2:1', api.nvim_exec('bdelete Xtestfile', true)) - eq({ 1, 1 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) + eq({ 1, 3 }, eval('[tabpagenr(), tabpagenr("$")]')) + eq('tabclosed:2:2:1\ntabclosed:2:2:1', exec_capture('bdelete Xtestfile')) + eq({ 1, 1 }, eval('[tabpagenr(), tabpagenr("$")]')) end) it('is triggered when closing a window via bdelete from current tab', function() @@ -46,9 +46,9 @@ describe('TabClosed', function() command('1tabedit Xtestfile2') -- Only one tab is closed, and the alternate file is used for the other. - eq({ 2, 3 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) - eq('tabclosed:2:2:2', api.nvim_exec('bdelete Xtestfile2', true)) - eq('Xtestfile1', api.nvim_eval('bufname("")')) + eq({ 2, 3 }, eval('[tabpagenr(), tabpagenr("$")]')) + eq('tabclosed:2:2:2', exec_capture('bdelete Xtestfile2')) + eq('Xtestfile1', eval('bufname("")')) end) it('triggers after tab page is properly freed', function() @@ -90,11 +90,11 @@ describe('TabClosed', function() command('au! TabClosed 2 echom "tabclosed:match"') repeat command('tabnew') - until api.nvim_eval('tabpagenr()') == 7 -- current tab is now 7 + until eval('tabpagenr()') == 7 -- current tab is now 7 -- sanity check, we shouldn't match on tabs with numbers other than 2 - eq('tabclosed:7:7:6', api.nvim_exec('tabclose', true)) + eq('tabclosed:7:7:6', exec_capture('tabclose')) -- close tab page 2, current tab is now 5 - eq('tabclosed:2:2:5\ntabclosed:match', api.nvim_exec('2tabclose', true)) + eq('tabclosed:2:2:5\ntabclosed:match', exec_capture('2tabclose')) end) end) @@ -104,9 +104,9 @@ describe('TabClosed', function() 'au! TabClosed * echom "tabclosed:".expand("").":".expand("").":".tabpagenr()' ) command('tabedit Xtestfile') - eq({ 2, 2 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) - eq('tabclosed:2:2:1', api.nvim_exec('close', true)) - eq({ 1, 1 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) + eq({ 2, 2 }, eval('[tabpagenr(), tabpagenr("$")]')) + eq('tabclosed:2:2:1', exec_capture('close')) + eq({ 1, 1 }, eval('[tabpagenr(), tabpagenr("$")]')) end) end) end) 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 04/10] 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() From 7896fe2deaa763df60c92442212fad86e8b3f292 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed, 16 Jul 2025 21:41:23 +0100 Subject: [PATCH 05/10] fix(api): do not allow opening float to closing buffer Problem: no check for nvim_open_win opening a new floating window into a closing buffer, which can lead to crashes. Solution: call check_split_disallowed; opening a new float semantically splits from a window, so the same problems as regular splits apply. Also restore the error if switch_win_noblock in win_set_buf fails (may not be possible to hit this, but win_set_buf can silently succeed there since #31595). As the lock check applies to curbuf (not the target buffer) this may feel a bit restrictive, though this isn't different to how things like ":split" or nvim_open_win with "split = true" works when targeting a different buffer. Only checking the target buffer's lock will cause issues if win_set_buf doesn't end up in the target buffer for whatever reason. Maybe we could consider checking the lock of the buffer after win_set_buf and close the window if it's locked (maybe a bit fiddly, especially as closing a window can fail...), or make the open + switch operation more atomic, like how Vim does for its popup windows..? It also used to be the case that win_set_buf would set an error if autocommands sent us to a different buffer than what was requested, but #31595 appears to have also changed that... I haven't touched that here. --- src/nvim/api/win_config.c | 3 +++ src/nvim/window.c | 7 ++++++- test/functional/api/window_spec.lua | 32 +++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index d64b9085e3..b4453ce4fb 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -290,6 +290,9 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Err } } } else { + if (!check_split_disallowed_err(curwin, err)) { + goto cleanup; // error already set + } wp = win_new_float(NULL, false, fconfig, err); } if (!wp) { diff --git a/src/nvim/window.c b/src/nvim/window.c index 1fec341045..69c343e94b 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -754,15 +754,17 @@ static void cmd_with_count(char *cmd, char *bufp, size_t bufsize, int64_t Prenum void win_set_buf(win_T *win, buf_T *buf, Error *err) FUNC_ATTR_NONNULL_ALL { + const handle_T win_handle = win->handle; tabpage_T *tab = win_find_tabpage(win); // no redrawing and don't set the window title RedrawingDisabled++; switchwin_T switchwin; + int win_result; TRY_WRAP(err, { - int win_result = switch_win_noblock(&switchwin, win, tab, true); + win_result = switch_win_noblock(&switchwin, win, tab, true); if (win_result != FAIL) { const int save_acd = p_acd; if (!switchwin.sw_same_win) { @@ -777,6 +779,9 @@ void win_set_buf(win_T *win, buf_T *buf, Error *err) } } }); + if (win_result == FAIL && !ERROR_SET(err)) { + api_set_error(err, kErrorTypeException, "Failed to switch to window %d", win_handle); + } // If window is not current, state logic will not validate its cursor. So do it now. // Still needed if do_buffer returns FAIL (e.g: autocmds abort script after buffer was set). diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 442f6e86e7..d5360e9fed 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -1803,6 +1803,38 @@ describe('API/win', function() .. '})' ) command('new | quit') + + -- Apply to opening floats too, as that can similarly create new views into a closing buffer. + -- For example, the following would open a float into an unloaded buffer: + exec([[ + only + new + let g:buf = bufnr() + autocmd BufUnload * ++once let g:win = nvim_open_win(g:buf, 0, #{relative: "editor", width: 5, height: 5, row: 1, col: 1}) + setlocal bufhidden=unload + ]]) + matches('E1159: Cannot split a window when closing the buffer$', pcall_err(command, 'quit')) + eq(false, eval('nvim_buf_is_loaded(g:buf)')) + eq(0, eval('win_findbuf(g:buf)->len()')) + + -- Only checking b_locked_split for the target buffer is insufficient, as naughty autocommands + -- can cause win_set_buf to remain in a closing curbuf: + exec([[ + only + new + let g:buf = bufnr() + autocmd BufWipeout * ++once ++nested let g:buf2 = nvim_create_buf(1, 0) + \| execute 'autocmd BufLeave * ++once call nvim_buf_delete(g:buf2, #{force: 1})' + \| setlocal bufhidden= + \| call nvim_open_win(g:buf2, 1, #{relative: 'editor', width: 5, height: 5, col: 5, row: 5}) + setlocal bufhidden=wipe + ]]) + matches('E1159: Cannot split a window when closing the buffer$', pcall_err(command, 'quit')) + eq(false, eval('nvim_buf_is_loaded(g:buf)')) + eq(0, eval('win_findbuf(g:buf)->len()')) + -- BufLeave shouldn't run here (buf2 isn't deleted and remains hidden) + eq(true, eval('nvim_buf_is_loaded(g:buf2)')) + eq(0, eval('win_findbuf(g:buf2)->len()')) end) it('restores last known cursor position if BufWinEnter did not move it', function() From 7a9bced0711019d0114b9b14d209ca8a0091ce14 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 22 Jul 2025 01:28:45 +0100 Subject: [PATCH 06/10] fix(window): disallow closing autocmd window in other tabpage Problem: unlike win_close, win_close_othertab could be used to close the autocommand window from a different tabpage. This causes aucmd_restbuf to close the wrong window, potentially causing a crash. Solution: disallow closing it. Also replace a deprecated use of exc_exec in the test file. Fixes #21409. --- src/nvim/window.c | 4 ++++ test/functional/autocmd/autocmd_spec.lua | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/nvim/window.c b/src/nvim/window.c index 69c343e94b..7324af4995 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2997,6 +2997,10 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) || (win->w_buffer != NULL && win->w_buffer->b_locked > 0)) { return false; // window is already being closed } + if (is_aucmd_win(win)) { + emsg(_(e_autocmd_close)); + return false; + } // Check if closing this window would leave only floating windows. if (tp->tp_firstwin == win && win->w_next && win->w_next->w_floating) { diff --git a/test/functional/autocmd/autocmd_spec.lua b/test/functional/autocmd/autocmd_spec.lua index fdf07a2d39..b4317a3e50 100644 --- a/test/functional/autocmd/autocmd_spec.lua +++ b/test/functional/autocmd/autocmd_spec.lua @@ -17,7 +17,6 @@ local pcall_err = t.pcall_err local fn = n.fn local expect = n.expect local command = n.command -local exc_exec = n.exc_exec local exec_lua = n.exec_lua local retry = t.retry local source = n.source @@ -151,7 +150,10 @@ describe('autocmd', function() }) command('autocmd BufLeave * bwipeout yy') - eq('Vim(edit):E143: Autocommands unexpectedly deleted new buffer yy', exc_exec('edit yy')) + eq( + 'Vim(edit):E143: Autocommands unexpectedly deleted new buffer yy', + pcall_err(command, 'edit yy') + ) expect([[ start of test file xx @@ -429,6 +431,23 @@ describe('autocmd', function() ) end) + it('cannot close `aucmd_win` in non-current tabpage', function() + exec([[ + file Xa + tabnew Xb + call setline(1, 'foo') + tabfirst + autocmd BufWriteCmd Xb tablast | bwipe! Xa + ]]) + eq( + 'BufWriteCmd Autocommands for "Xb": Vim(bwipeout):E813: Cannot close autocmd window', + pcall_err(command, 'wall') + ) + -- Sanity check: :bwipe failing to close all windows into Xa should keep it loaded. + -- (So there's no risk of it being left unloaded inside a window) + eq(1, eval('bufloaded("Xa")')) + end) + describe('closing last non-floating window in tab from `aucmd_win`', function() before_each(function() command('edit Xa.txt') From 7f51431c12ef44fdfddc5d15b2ece3e1a30ffaf3 Mon Sep 17 00:00:00 2001 From: glepnir Date: Tue, 9 Sep 2025 09:30:20 +0800 Subject: [PATCH 07/10] fix(api): crash when moving curwin to other tabpage #35679 Problem: nvim_win_set_config may crash when attempting to move curwin to a different tabpage if there is no other non-float available to switch to. Solution: fix the crash. Fix ONE_WINDOW checks in winframe_find_altwin and win_altframe to consider floating windows by instead using one_window. Allow one_window to consider non-current tabpages. We can use one_window in win_close_othertab now to also better reflect its use in win_close. Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com> --- src/nvim/api/win_config.c | 11 +++++-- src/nvim/buffer.c | 4 +-- src/nvim/version.c | 2 +- src/nvim/window.c | 45 +++++++++++++++-------------- test/functional/api/window_spec.lua | 35 ++++++++++++++++++++-- 5 files changed, 69 insertions(+), 28 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index b4453ce4fb..6a7baaab46 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -492,7 +492,14 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) if (curwin_moving_tp) { if (was_split) { int dir; - win_goto(winframe_find_altwin(win, &dir, NULL, NULL)); + win_T *altwin = winframe_find_altwin(win, &dir, NULL, NULL); + // Autocommands may still make this the last non-float after this check. + // That case will be caught later when trying to move the window. + if (!altwin) { + api_set_error(err, kErrorTypeException, "Cannot move last non-floating window"); + return; + } + win_goto(altwin); } else { win_goto(win_float_find_altwin(win, NULL)); } @@ -525,7 +532,7 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) // FIXME(willothy): if the window is the last in the tabpage but there is another tabpage // and the target window is in that other tabpage, should we move the window to that // tabpage and close the previous one, or just error? - api_set_error(err, kErrorTypeException, "Cannot move last window"); + api_set_error(err, kErrorTypeException, "Cannot move last non-floating window"); goto restore_curwin; } else if (parent != NULL && parent->handle == win->handle) { int n_frames = 0; diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index aa1a8b2df9..4d46a3ec7c 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -571,7 +571,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i } buf->b_locked--; buf->b_locked_split--; - if (abort_if_last && one_window(win)) { + if (abort_if_last && win != NULL && one_window(win, NULL)) { // Autocommands made this the only window. emsg(_(e_auabort)); return false; @@ -590,7 +590,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i } buf->b_locked--; buf->b_locked_split--; - if (abort_if_last && one_window(win)) { + if (abort_if_last && win != NULL && one_window(win, NULL)) { // Autocommands made this the only window. emsg(_(e_auabort)); return false; diff --git a/src/nvim/version.c b/src/nvim/version.c index 755288f137..400a9719bf 100644 --- a/src/nvim/version.c +++ b/src/nvim/version.c @@ -2720,7 +2720,7 @@ bool may_show_intro(void) && (curbuf->b_fname == NULL) && (curbuf->handle == 1) && (curwin->handle == LOWEST_WIN_ID) - && one_window(curwin) + && one_window(curwin, NULL) && (vim_strchr(p_shm, SHM_INTRO) == NULL)); } diff --git a/src/nvim/window.c b/src/nvim/window.c index 7324af4995..cb102926d4 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -427,7 +427,7 @@ newwindow: // move window to new tab page case 'T': CHECK_CMDWIN; - if (one_window(curwin)) { + if (one_window(curwin, NULL)) { msg(_(m_onlyone), 0); } else { tabpage_T *oldtab = curtab; @@ -500,7 +500,7 @@ newwindow: case 'H': case 'L': CHECK_CMDWIN; - if (one_window(curwin)) { + if (one_window(curwin, NULL)) { beep_flush(); } else { const int dir = ((nchar == 'H' || nchar == 'L') ? WSP_VERT : 0) @@ -1086,7 +1086,7 @@ win_T *win_split_ins(int size, int flags, win_T *new_wp, int dir, frame_T *to_fl bool toplevel = flags & (WSP_TOP | WSP_BOT); // add a status line when p_ls == 1 and splitting the first window - if (one_window(firstwin) && p_ls == 1 && oldwin->w_status_height == 0) { + if (one_window(firstwin, NULL) && p_ls == 1 && oldwin->w_status_height == 0) { if (oldwin->w_height <= p_wmh) { emsg(_(e_noroom)); return NULL; @@ -1796,7 +1796,7 @@ static void win_exchange(int Prenum) return; } - if (one_window(curwin)) { + if (one_window(curwin, NULL)) { // just one window beep_flush(); return; @@ -1888,7 +1888,7 @@ static void win_rotate(bool upwards, int count) return; } - if (count <= 0 || one_window(curwin)) { + if (count <= 0 || one_window(curwin, NULL)) { // nothing to do beep_flush(); return; @@ -1971,7 +1971,7 @@ int win_splitmove(win_T *wp, int size, int flags) int dir = 0; int height = wp->w_height; - if (one_window(wp)) { + if (one_window(wp, NULL)) { return OK; // nothing to do } if (is_aucmd_win(wp) || check_split_disallowed(wp) == FAIL) { @@ -2500,7 +2500,7 @@ void close_windows(buf_T *buf, bool keep_curwin) // Start from lastwin to close floating windows with the same buffer first. // When the autocommand window is involved win_close() may need to print an error message. - for (win_T *wp = lastwin; wp != NULL && (is_aucmd_win(lastwin) || !one_window(wp));) { + for (win_T *wp = lastwin; wp != NULL && (is_aucmd_win(lastwin) || !one_window(wp, NULL));) { if (wp->w_buffer == buf && (!keep_curwin || wp != curwin) && !(win_locked(wp) || wp->w_buffer->b_locked > 0)) { if (win_close(wp, false, false) == FAIL) { @@ -2545,17 +2545,19 @@ void close_windows(buf_T *buf, bool keep_curwin) /// Check if "win" is the last non-floating window that exists. bool last_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT { - return one_window(win) && first_tabpage->tp_next == NULL; + return one_window(win, NULL) && first_tabpage->tp_next == NULL; } -/// Check if "win" is the only non-floating window in the current tabpage. +/// Check if "win" is the only non-floating window in tabpage "tp", or NULL for current tabpage. /// /// This should be used in place of ONE_WINDOW when necessary, /// with "firstwin" or the affected window as argument depending on the situation. -bool one_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT +bool one_window(win_T *win, tabpage_T *tp) + FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ARG(1) { - assert(!firstwin->w_floating); - return firstwin == win && (win->w_next == NULL || win->w_next->w_floating); + win_T *first = tp ? tp->tp_firstwin : firstwin; + assert((!tp || tp != curtab) && !first->w_floating); + return first == win && (win->w_next == NULL || win->w_next->w_floating); } /// Check if floating windows in tabpage `tp` can be closed. @@ -2706,7 +2708,7 @@ int win_close(win_T *win, bool free_buf, bool force) emsg(_(e_autocmd_close)); return FAIL; } - if (lastwin->w_floating && one_window(win)) { + if (lastwin->w_floating && one_window(win, NULL)) { if (is_aucmd_win(lastwin)) { emsg(_("E814: Cannot close window, only autocmd window would remain")); return FAIL; @@ -3003,7 +3005,7 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) } // Check if closing this window would leave only floating windows. - if (tp->tp_firstwin == win && win->w_next && win->w_next->w_floating) { + if (tp->tp_lastwin->w_floating && one_window(win, tp)) { if (force || can_close_floating_windows(tp)) { // close the last window until the there are no floating windows while (tp->tp_lastwin->w_floating) { @@ -3053,7 +3055,7 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) } // Autocommands may again cause closing this window to leave only floats. // Check again; we'll not bother closing floating windows this time. - if (tp->tp_firstwin == win && win->w_next && win->w_next->w_floating) { + if (tp->tp_lastwin->w_floating && one_window(win, tp)) { emsg(e_floatonly); goto leave_open; } @@ -3262,14 +3264,15 @@ win_T *winframe_remove(win_T *win, int *dirp, tabpage_T *tp, frame_T **unflat_al /// @param tp tab page "win" is in, NULL for current /// @param altfr if not NULL, set to pointer of frame that will get the space /// -/// @return a pointer to the window that will get the freed up space. +/// @return a pointer to the window that will get the freed up space, or NULL +/// if there is no other non-float to receive the space. win_T *winframe_find_altwin(win_T *win, int *dirp, tabpage_T *tp, frame_T **altfr) FUNC_ATTR_NONNULL_ARG(1, 2) { assert(tp == NULL || tp != curtab); - // If there is only one window there is nothing to remove. - if (tp == NULL ? ONE_WINDOW : tp->tp_firstwin == tp->tp_lastwin) { + // If there is only one non-floating window there is nothing to remove. + if (one_window(win, tp)) { return NULL; } @@ -3466,7 +3469,7 @@ static frame_T *win_altframe(win_T *win, tabpage_T *tp) { assert(tp == NULL || tp != curtab); - if (tp == NULL ? ONE_WINDOW : tp->tp_firstwin == tp->tp_lastwin) { + if (one_window(win, tp)) { return alt_tabpage()->tp_curwin->w_frame; } @@ -4035,7 +4038,7 @@ void close_others(int message, int forceit) return; } - if (one_window(firstwin) && !lastwin->w_floating) { + if (one_window(firstwin, NULL) && !lastwin->w_floating) { if (message && !autocmd_busy) { msg(_(m_onlyone), 0); @@ -7092,7 +7095,7 @@ int global_stl_height(void) /// @param morewin pretend there are two or more windows if true. int last_stl_height(bool morewin) { - return (p_ls > 1 || (p_ls == 1 && (morewin || !one_window(firstwin)))) ? STATUS_HEIGHT : 0; + return (p_ls > 1 || (p_ls == 1 && (morewin || !one_window(firstwin, NULL)))) ? STATUS_HEIGHT : 0; } /// Return the minimal number of rows that is needed on the screen to display diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index d5360e9fed..03df326781 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2111,11 +2111,42 @@ describe('API/win', function() eq('editor', api.nvim_win_get_config(win).relative) end) - it('throws error when attempting to move the last window', function() + it('throws error when attempting to move the last non-floating window', function() local err = pcall_err(api.nvim_win_set_config, 0, { vertical = false, }) - eq('Cannot move last window', err) + eq('Cannot move last non-floating window', err) + + local win1 = api.nvim_get_current_win() + command('tabnew') + eq( + 'Cannot move last non-floating window', + pcall_err(api.nvim_win_set_config, 0, { win = win1, split = 'left' }) + ) + api.nvim_open_win(0, false, { relative = 'editor', width = 5, height = 5, row = 1, col = 1 }) + eq( + 'Cannot move last non-floating window', + pcall_err(api.nvim_win_set_config, 0, { win = win1, split = 'left' }) + ) + + -- If it's no longer the last non-float, still an error if autocommands make it the last + -- non-float again before it's moved. + command('vsplit') + exec_lua(function() + vim.api.nvim_create_autocmd('WinEnter', { + once = true, + callback = function() + vim.api.nvim_win_set_config( + 0, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + end, + }) + end) + eq( + 'Cannot move last non-floating window', + pcall_err(api.nvim_win_set_config, 0, { win = win1, split = 'left' }) + ) end) it('passing retval of get_config results in no-op', function() From 91ebbc6c4e4e94da0d1c38a297d04173f27bdbf8 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed, 17 Dec 2025 16:38:52 +0000 Subject: [PATCH 08/10] fix(api): ignore split_disallowed when opening a float Problem: split_disallowed seemingly exists to prevent issues from changing frames to accomodate a split window, which doesn't apply to floats. Solution: remove the restriction for nvim_open_win, but only for floats. (continue to check b_locked_split though) NOTE: like before, the buffer we check b_locked_split for may not actually be the target buffer "buf", as the later call to win_set_buf can fail to switch to "buf" due to autocommands. (among other things) Maybe we could attempt to close the new window in that case (or switch to a different buffer if that also fails), but this is safer. (and simpler) Fixes #36857 (and possibly some spurious E242s I've observed from extui) --- src/nvim/api/win_config.c | 8 ++++++-- test/functional/api/window_spec.lua | 13 ++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 6a7baaab46..b7a00adbc8 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -290,8 +290,12 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Err } } } else { - if (!check_split_disallowed_err(curwin, err)) { - goto cleanup; // error already set + // Unlike check_split_disallowed_err, ignore `split_disallowed`, as opening a float shouldn't + // mess with the frame structure. Still check `b_locked_split` to avoid opening more windows + // into a closing buffer, though. + if (curwin->w_buffer->b_locked_split) { // Can't instead check `buf` in case win_set_buf fails! + api_set_error(err, kErrorTypeException, "E1159: Cannot open a float when closing the buffer"); + goto cleanup; } wp = win_new_float(NULL, false, fconfig, err); } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 03df326781..7ec2288e21 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -1777,6 +1777,13 @@ describe('API/win', function() it('checks if splitting disallowed', function() command('split | autocmd WinEnter * ++once call nvim_open_win(0, 0, #{split: "right"})') matches("E242: Can't split a window while closing another$", pcall_err(command, 'quit')) + -- E242 is not needed for floats. + exec([[ + split + autocmd WinEnter * ++once let g:win = nvim_open_win(0, 0, #{relative: "editor", row: 0, col: 0, width: 5, height: 5}) + quit + ]]) + eq('editor', eval('nvim_win_get_config(g:win).relative')) command('only | autocmd BufHidden * ++once call nvim_open_win(0, 0, #{split: "left"})') matches( @@ -1810,10 +1817,10 @@ describe('API/win', function() only new let g:buf = bufnr() - autocmd BufUnload * ++once let g:win = nvim_open_win(g:buf, 0, #{relative: "editor", width: 5, height: 5, row: 1, col: 1}) + autocmd BufUnload * ++once call nvim_open_win(g:buf, 0, #{relative: "editor", width: 5, height: 5, row: 1, col: 1}) setlocal bufhidden=unload ]]) - matches('E1159: Cannot split a window when closing the buffer$', pcall_err(command, 'quit')) + matches('E1159: Cannot open a float when closing the buffer$', pcall_err(command, 'quit')) eq(false, eval('nvim_buf_is_loaded(g:buf)')) eq(0, eval('win_findbuf(g:buf)->len()')) @@ -1829,7 +1836,7 @@ describe('API/win', function() \| call nvim_open_win(g:buf2, 1, #{relative: 'editor', width: 5, height: 5, col: 5, row: 5}) setlocal bufhidden=wipe ]]) - matches('E1159: Cannot split a window when closing the buffer$', pcall_err(command, 'quit')) + matches('E1159: Cannot open a float when closing the buffer$', pcall_err(command, 'quit')) eq(false, eval('nvim_buf_is_loaded(g:buf)')) eq(0, eval('win_findbuf(g:buf)->len()')) -- BufLeave shouldn't run here (buf2 isn't deleted and remains hidden) From a9ffdca528d132c0a9016495c2e62eb1d4563bfc Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed, 17 Dec 2025 17:57:30 +0000 Subject: [PATCH 09/10] fix(api): open_win leak from naughty autocommands Problem: error set by win_set_buf may leak if autocommands immediately close the new window. Solution: free the error set by win_set_buf. (prefer nvim_open_win's error as it's more important and will cause 0 to be returned) --- src/nvim/api/win_config.c | 1 + test/functional/api/window_spec.lua | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index b7a00adbc8..2af6e56a3d 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -343,6 +343,7 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Err } } if (!tp) { + api_clear_error(err); // may have been set by win_set_buf api_set_error(err, kErrorTypeException, "Window was closed immediately"); goto cleanup; } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 7ec2288e21..400c3611c7 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2068,6 +2068,31 @@ describe('API/win', function() api.nvim_open_win(0, true, { split = 'below' }) eq(11, api.nvim_win_get_height(0)) end) + + it('no leak when win_set_buf fails and window is closed immediately', function() + -- Following used to leak. + command('autocmd BufEnter * ++once quit! | throw 1337') + eq( + 'Window was closed immediately', + pcall_err( + api.nvim_open_win, + api.nvim_create_buf(true, true), + true, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + ) + -- If the window wasn't closed, still set errors from win_set_buf. + command('autocmd BufEnter * ++once throw 1337') + eq( + 'BufEnter Autocommands for "*": 1337', + pcall_err( + api.nvim_open_win, + api.nvim_create_buf(true, true), + true, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + ) + end) end) describe('set_config', function() From f7e2554bfb174919ad6e6c2ce25f8681060ab832 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 5 Jan 2026 11:17:00 +0800 Subject: [PATCH 10/10] fix(window): crash closing split if autocmds close other splits (#37233) Problem: Crash when closing a split window if autocmds close other split windows but there are still floating windows. Solution: Bail out and give the window back its buffer. --- src/nvim/window.c | 57 +++++++++++++++++++++++-------- test/functional/ui/float_spec.lua | 54 +++++++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 17 deletions(-) diff --git a/src/nvim/window.c b/src/nvim/window.c index cb102926d4..9e36d93953 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2650,7 +2650,9 @@ static bool close_last_window_tabpage(win_T *win, bool free_buf, tabpage_T *prev /// "action" can also be zero (do nothing). /// "abort_if_last" is passed to close_buffer(): abort closing if all other /// windows are closed. -static void win_close_buffer(win_T *win, int action, bool abort_if_last) +/// +/// @return whether close_buffer() decremented b_nwindows +static bool win_close_buffer(win_T *win, int action, bool abort_if_last) FUNC_ATTR_NONNULL_ALL { // Free independent synblock before the buffer is freed. @@ -2665,12 +2667,13 @@ static void win_close_buffer(win_T *win, int action, bool abort_if_last) win->w_buffer->b_p_bl = false; } + bool retval = false; // Close the link to the buffer. if (win->w_buffer != NULL) { bufref_T bufref; set_bufref(&bufref, curbuf); win->w_locked = true; - close_buffer(win, win->w_buffer, action, abort_if_last, true); + retval = close_buffer(win, win->w_buffer, action, abort_if_last, true); if (win_valid_any_tab(win)) { win->w_locked = false; } @@ -2681,6 +2684,27 @@ static void win_close_buffer(win_T *win, int action, bool abort_if_last) curbuf = firstbuf; } } + + return retval; +} + +/// When failing to close a window after already calling close_buffer() on it, +/// call this to make the window have a buffer again. +/// +/// @param bufref reference to win->w_buffer before calling close_buffer() +/// @param did_decrement whether close_buffer() decremented b_nwindows +static void win_unclose_buffer(win_T *win, bufref_T *bufref, bool did_decrement) +{ + 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++; + } } // Close window "win". Only works for the current tab page. @@ -2804,7 +2828,10 @@ int win_close(win_T *win, bool free_buf, bool force) return OK; } - win_close_buffer(win, free_buf ? DOBUF_UNLOAD : 0, true); + bufref_T bufref; + set_bufref(&bufref, win->w_buffer); + + bool did_decrement = win_close_buffer(win, free_buf ? DOBUF_UNLOAD : 0, true); if (win_valid(win) && win->w_buffer == NULL && !win->w_floating && last_window(win)) { @@ -2825,8 +2852,17 @@ int win_close(win_T *win, bool free_buf, bool force) // Autocommands may have closed the window already, or closed the only // other window or moved to another tab page. - if (!win_valid(win) || (!win->w_floating && last_window(win)) - || close_last_window_tabpage(win, free_buf, prev_curtab)) { + if (!win_valid(win)) { + return FAIL; + } + if (one_window(win, NULL) && (first_tabpage->tp_next == NULL || lastwin->w_floating)) { + if (first_tabpage->tp_next != NULL) { + emsg(e_floatonly); + } + win_unclose_buffer(win, &bufref, did_decrement); + return FAIL; + } + if (close_last_window_tabpage(win, free_buf, prev_curtab)) { return FAIL; } @@ -3105,16 +3141,7 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) leave_open: 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++; - } + win_unclose_buffer(win, &bufref, did_decrement); } return false; } diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index c8492b0f3e..34d7d7be7a 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -1005,7 +1005,7 @@ describe('float window', function() assert_alive() end) - pending('does not crash if BufUnload makes it the only non-float in tabpage', function() + it('does not crash if BufUnload makes it the only non-float in tabpage', function() exec([[ tabnew let g:buf = bufnr() @@ -1017,10 +1017,60 @@ describe('float window', function() \ #{relative: 'editor', row: 5, col: 5, width: 5, height: 5}) autocmd BufUnload * ++once exe g:buf .. 'bwipe!' ]]) - command('close') + eq('Vim(close):E5601: Cannot close window, only floating window would remain', pcall_err(command, 'close')) assert_alive() end) + describe('does not crash if WinClosed makes it the only non-float', function() + before_each(function() + exec([[ + let g:buf = bufnr() + new + setlocal bufhidden=wipe + autocmd WinClosed * ++once exe g:buf .. 'bwipe!' + ]]) + end) + + local opts = { relative = 'editor', row = 5, col = 5, width = 5, height = 5 } + local floatwin + + describe('and there is a float window with the same buffer', function() + before_each(function() + floatwin = api.nvim_open_win(0, false, opts) + end) + + it('with multiple tabpages', function() + command('tabnew | tabprev') + eq('Vim(close):E5601: Cannot close window, only floating window would remain', pcall_err(command, 'close')) + api.nvim_win_close(floatwin, true) + assert_alive() + end) + + it('with only one tabpage', function() + command('close') + api.nvim_win_close(floatwin, true) + assert_alive() + end) + end) + + describe('and there is a float with a different buffer', function() + before_each(function() + floatwin = api.nvim_open_win(api.nvim_create_buf(true, false), false, opts) + end) + + it('with multiple tabpages', function() + command('tabnew | tabprev') + eq('Vim(close):E855: Autocommands caused command to abort', pcall_err(command, 'close')) + assert_alive() + end) + + it('with only one tabpage', function() + eq('Vim(close):E855: Autocommands caused command to abort', pcall_err(command, 'close')) + assert_alive() + end) + end) + end) + it('does not crash if WinClosed from floating window closes it', function() exec([[ tabnew