fix(api): on_detach consistently before buf_freeall autocmds (#35369)

Problem: on_detach may be called after buf_freeall and other important things,
plus its textlock restrictions are insufficient. This can cause issues such as
leaks, internal errors and crashes.

Solution: disable buffer updates in buf_freeall, before autocommands (like the
order after #35355 and when do_ecmd reloads a buffer). Don't do so in
free_buffer_stuff; it's not safe to run user code there, and buf_freeall already
runs before then; just free them to avoid leaks if buf_freeall autocommands
registered more for some reason.

(cherry picked from commit 2211953266)

Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
This commit is contained in:
neovim-backports[bot]
2025-08-17 15:39:32 -07:00
committed by GitHub
parent 37b2d42459
commit 3ab06d5188
3 changed files with 139 additions and 42 deletions

View File

@@ -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.

View File

@@ -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

View File

@@ -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<integer, string[]>
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! \<C-W>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)