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] 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()