diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index fba19ba9f0..a923a4bbe1 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -657,10 +657,6 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i buf->b_nwindows = nwindows; - // Disable buffer-updates for the current buffer. - // No need to check `unload_buf`: in that case the function returned above. - buf_updates_unload(buf, false); - buf_freeall(buf, ((del_buf ? BFA_DEL : 0) + (wipe_buf ? BFA_WIPE : 0) + (ignore_abort ? BFA_IGNORE_ABORT : 0))); @@ -789,6 +785,11 @@ void buf_freeall(buf_T *buf, int flags) bufref_T bufref; set_bufref(&bufref, buf); + buf_updates_unload(buf, false); + if (!bufref_valid(&bufref)) { + // on_detach callback deleted the buffer. + return; + } if ((buf->b_ml.ml_mfp != NULL) && apply_autocmds(EVENT_BUFUNLOAD, buf->b_fname, buf->b_fname, false, buf) && !bufref_valid(&bufref)) { @@ -935,7 +936,7 @@ static void free_buffer_stuff(buf_T *buf, int free_flags) map_clear_mode(buf, MAP_ALL_MODES, true, true); // clear local abbrevs XFREE_CLEAR(buf->b_start_fenc); - buf_updates_unload(buf, false); + buf_free_callbacks(buf); } /// Go to another buffer. Handles the result of the ATTENTION dialog. diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c index fd77f108af..aa07d30a4f 100644 --- a/src/nvim/ex_cmds.c +++ b/src/nvim/ex_cmds.c @@ -2512,13 +2512,11 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum goto theend; } u_unchanged(curbuf); - buf_updates_unload(curbuf, false); buf_freeall(curbuf, BFA_KEEP_UNDO); // Tell readfile() not to clear or reload undo info. readfile_flags = READ_KEEP_UNDO; } else { - buf_updates_unload(curbuf, false); buf_freeall(curbuf, 0); // Free all things for buffer. } // If autocommands deleted the buffer we were going to re-edit, give diff --git a/test/functional/lua/buffer_updates_spec.lua b/test/functional/lua/buffer_updates_spec.lua index eb3deadc10..5045d3e72f 100644 --- a/test/functional/lua/buffer_updates_spec.lua +++ b/test/functional/lua/buffer_updates_spec.lua @@ -14,6 +14,8 @@ local feed = n.feed local expect_events = t.expect_events local write_file = t.write_file local dedent = t.dedent +local matches = t.matches +local pcall_err = t.pcall_err local origlines = { 'original line 1', @@ -1386,42 +1388,138 @@ describe('lua: nvim_buf_attach on_bytes', function() end) describe('nvim_buf_attach on_detach', function() - it('is invoked before unloading buffer', function() + it('called before buf_freeall autocommands', function() exec_lua(function() - _G.logs = {} ---@type table + vim.api.nvim_create_autocmd({ 'BufUnload', 'BufDelete', 'BufWipeout' }, { + callback = function(args) + table.insert( + _G.events, + ('%s: %d %s'):format( + args.event, + args.buf, + tostring(vim.api.nvim_buf_is_loaded(args.buf)) + ) + ) + end, + }) + function _G.on_detach(_, b) + table.insert( + _G.events, + ('on_detach: %d %s'):format(b, tostring(vim.api.nvim_buf_is_loaded(b))) + ) + end + _G.events = {} + vim.cmd 'new' + vim.bo.bufhidden = 'wipe' + vim.api.nvim_buf_attach(0, false, { on_detach = _G.on_detach }) + vim.cmd 'quit!' end) - local function setup(bufnr) - exec_lua(function() - _G.logs[bufnr] = {} - vim.api.nvim_create_autocmd({ 'BufUnload', 'BufWipeout' }, { - buffer = bufnr, - callback = function(ev) - table.insert(_G.logs[bufnr], ev.event) - end, - }) - vim.api.nvim_buf_attach(bufnr, false, { - on_detach = function() - table.insert(_G.logs[bufnr], 'on_detach') - end, - }) - end) - end - -- Test with two buffers because the :bw works differently for the last buffer. - -- Before #35355, the order was as follows: - -- * non-last buffers: BufUnload → BufWipeout → on_detach - -- * the last buffer (with text): BufUnload → on_detach → BufWipeout - local buf1 = api.nvim_get_current_buf() - local buf2 = api.nvim_create_buf(true, false) - api.nvim_open_win(buf2, false, { split = 'below' }) - api.nvim_buf_set_lines(buf1, 0, -1, true, { 'abc' }) - api.nvim_buf_set_lines(buf2, 0, -1, true, { 'abc' }) - setup(buf1) - setup(buf2) - api.nvim_buf_delete(buf1, { force = true }) - api.nvim_buf_delete(buf2, { force = true }) - local logs = exec_lua('return _G.logs') - local order = { 'on_detach', 'BufUnload', 'BufWipeout' } - eq(order, logs[buf1]) - eq(order, logs[buf2]) + + eq( + { 'on_detach: 2 true', 'BufUnload: 2 true', 'BufDelete: 2 true', 'BufWipeout: 2 true' }, + exec_lua('return _G.events') + ) + eq(false, api.nvim_buf_is_valid(2)) + + exec_lua(function() + _G.events = {} + local buf = vim.api.nvim_create_buf(false, true) + vim.api.nvim_buf_attach(buf, false, { on_detach = _G.on_detach }) + vim.api.nvim_buf_delete(buf, { force = true }) + end) + + -- Was unlisted, so no BufDelete. + eq( + { 'on_detach: 3 true', 'BufUnload: 3 true', 'BufWipeout: 3 true' }, + exec_lua('return _G.events') + ) + eq(false, api.nvim_buf_is_valid(3)) + + exec_lua(function() + _G.events = {} + vim.api.nvim_buf_attach(1, false, { on_detach = _G.on_detach }) + vim.api.nvim_create_autocmd('BufUnload', { + buffer = 1, + once = true, + callback = function() + vim.api.nvim_buf_attach(1, false, { + on_detach = function(...) + vim.fn.bufload(1) -- Leaks the memfile it were to run inside free_buffer_stuff. + return _G.on_detach(...) + end, + }) + table.insert(_G.events, 'local BufUnload') + end, + }) + vim.cmd 'edit asdf' -- Reuses buffer 1. + end) + + -- on_detach shouldn't run after autocommands when reusing a buffer (in free_buffer_stuff), even + -- if those autocommands registered it, as curbuf may be in a semi-unloaded state at that point. + eq({ + 'on_detach: 1 true', + 'BufUnload: 1 true', + 'local BufUnload', + 'BufDelete: 1 true', + 'BufWipeout: 1 true', + }, exec_lua('return _G.events')) + + exec_lua(function() + _G.events = {} + vim.api.nvim_buf_attach(0, false, { on_detach = _G.on_detach }) + vim.cmd 'edit' + end) + + -- Re-edit buffer; on_detach is called. + eq({ 'on_detach: 1 true', 'BufUnload: 1 true' }, exec_lua('return _G.events')) + eq(true, api.nvim_buf_is_valid(1)) + + exec_lua(function() + vim.cmd '%bwipeout!' + vim.bo.modified = true + _G.events = {} + vim.api.nvim_buf_attach(0, false, { on_detach = _G.on_detach }) + vim.api.nvim_buf_delete(0, { force = true }) + end) + + -- on_detach must still be first when wiping the last buffer if it's listed and non-reusable. + -- Previously: BufUnload → BufDelete → on_detach → BufWipeout. + eq( + { 'on_detach: 4 true', 'BufUnload: 4 true', 'BufDelete: 4 true', 'BufWipeout: 4 false' }, + exec_lua('return _G.events') + ) + end) + + it('disallows splitting', function() + command('new | setlocal bufhidden=wipe') + local buf = api.nvim_get_current_buf() + exec_lua(function() + vim.api.nvim_buf_attach(0, false, { + on_detach = function() + -- Used to allow opening more views into a closing buffer, resulting in open windows to an + -- unloaded buffer. + vim.cmd [=[execute "normal! \s"]=] + end, + }) + end) + matches('E1159: Cannot split a window when closing the buffer$', pcall_err(command, 'quit!')) + eq({}, fn.win_findbuf(buf)) + eq(false, api.nvim_buf_is_valid(buf)) end) end) + +it('nvim_buf_attach from buf_freeall autocommands does not leak', function() + exec_lua(function() + local b = vim.api.nvim_create_buf(true, true) + vim.api.nvim_create_autocmd('BufWipeout', { + buffer = b, + once = true, + callback = function() + vim.api.nvim_buf_attach(b, false, {}) + _G.autocmd_fired = true + end, + }) + vim.api.nvim_buf_delete(b, { force = true }) + end) + eq(true, exec_lua('return _G.autocmd_fired')) +end)