mirror of
https://github.com/neovim/neovim
synced 2025-07-16 17:21:49 +00:00
fix(autocmds): once=true Lua event-handler may call itself #29544
Problem: Event handler declared with `once=true` can re-trigger itself (i.e. more than once!) by calling `nvim_exec_autocmds` or `:doautocmd`. Analysis: This happens because the callback is executed before deletion/cleanup (`aucmd_del`). And calling `aucmd_del` before `call_autocmd_callback` breaks the autocmd execution... Solution: Set `ac->pat=NULL` to temporarily "delete" the autocmd, then restore it after executing the callback. Fix #25526 Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
This commit is contained in:
@ -2025,7 +2025,8 @@ static void aucmd_next(AutoPatCmd *apc)
|
|||||||
apc->auidx = SIZE_MAX;
|
apc->auidx = SIZE_MAX;
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool call_autocmd_callback(const AutoCmd *ac, const AutoPatCmd *apc)
|
/// Executes an autocmd callback function (as opposed to an Ex command).
|
||||||
|
static bool au_callback(const AutoCmd *ac, const AutoPatCmd *apc)
|
||||||
{
|
{
|
||||||
Callback callback = ac->exec.callable.cb;
|
Callback callback = ac->exec.callable.cb;
|
||||||
if (callback.type == kCallbackLua) {
|
if (callback.type == kCallbackLua) {
|
||||||
@ -2106,16 +2107,24 @@ char *getnextac(int c, void *cookie, int indent, bool do_concat)
|
|||||||
apc->script_ctx = current_sctx;
|
apc->script_ctx = current_sctx;
|
||||||
|
|
||||||
char *retval;
|
char *retval;
|
||||||
if (ac->exec.type == CALLABLE_CB) {
|
switch (ac->exec.type) {
|
||||||
// Can potentially reallocate kvec_t data and invalidate the ac pointer
|
case CALLABLE_EX:
|
||||||
if (call_autocmd_callback(ac, apc)) {
|
retval = xstrdup(ac->exec.callable.cmd);
|
||||||
// If an autocommand callback returns true, delete the autocommand
|
break;
|
||||||
oneshot = true;
|
case CALLABLE_CB: {
|
||||||
|
AutoCmd ac_copy = *ac;
|
||||||
|
// Mark oneshot handler as "removed" now, to prevent recursion by e.g. `:doautocmd`. #25526
|
||||||
|
ac->pat = oneshot ? NULL : ac->pat;
|
||||||
|
// May reallocate `acs` kvec_t data and invalidate the `ac` pointer.
|
||||||
|
bool rv = au_callback(&ac_copy, apc);
|
||||||
|
if (oneshot) {
|
||||||
|
// Restore `pat`. Use `acs` because `ac` may have been invalidated by the callback.
|
||||||
|
kv_A(*acs, apc->auidx).pat = ac_copy.pat;
|
||||||
}
|
}
|
||||||
|
// If an autocommand callback returns true, delete the autocommand
|
||||||
|
oneshot = oneshot || rv;
|
||||||
|
|
||||||
// TODO(tjdevries):
|
// HACK(tjdevries):
|
||||||
//
|
|
||||||
// Major Hack Alert:
|
|
||||||
// We just return "not-null" and continue going.
|
// We just return "not-null" and continue going.
|
||||||
// This would be a good candidate for a refactor. You would need to refactor:
|
// This would be a good candidate for a refactor. You would need to refactor:
|
||||||
// 1. do_cmdline to accept something besides a string
|
// 1. do_cmdline to accept something besides a string
|
||||||
@ -2124,8 +2133,11 @@ char *getnextac(int c, void *cookie, int indent, bool do_concat)
|
|||||||
// and instead we loop over all the matches and just execute one-by-one.
|
// and instead we loop over all the matches and just execute one-by-one.
|
||||||
// However, my expectation would be that could be expensive.
|
// However, my expectation would be that could be expensive.
|
||||||
retval = xcalloc(1, 1);
|
retval = xcalloc(1, 1);
|
||||||
} else {
|
break;
|
||||||
retval = xstrdup(ac->exec.callable.cmd);
|
}
|
||||||
|
case CALLABLE_NONE:
|
||||||
|
default:
|
||||||
|
abort();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Remove one-shot ("once") autocmd in anticipation of its execution.
|
// Remove one-shot ("once") autocmd in anticipation of its execution.
|
||||||
|
@ -160,7 +160,7 @@ describe('autocmd', function()
|
|||||||
|
|
||||||
it('++once', function() -- :help autocmd-once
|
it('++once', function() -- :help autocmd-once
|
||||||
--
|
--
|
||||||
-- ":autocmd ... ++once" executes its handler once, then removes the handler.
|
-- ":autocmd … ++once" executes its handler once, then removes the handler.
|
||||||
--
|
--
|
||||||
local expected = {
|
local expected = {
|
||||||
'Many1',
|
'Many1',
|
||||||
@ -206,7 +206,7 @@ describe('autocmd', function()
|
|||||||
)
|
)
|
||||||
|
|
||||||
--
|
--
|
||||||
-- ":autocmd ... ++once" handlers can be deleted.
|
-- ":autocmd … ++once" handlers can be deleted.
|
||||||
--
|
--
|
||||||
expected = {}
|
expected = {}
|
||||||
command('let g:foo = []')
|
command('let g:foo = []')
|
||||||
@ -216,7 +216,7 @@ describe('autocmd', function()
|
|||||||
eq(expected, eval('g:foo'))
|
eq(expected, eval('g:foo'))
|
||||||
|
|
||||||
--
|
--
|
||||||
-- ":autocmd ... <buffer> ++once ++nested"
|
-- ":autocmd … <buffer> ++once ++nested"
|
||||||
--
|
--
|
||||||
expected = {
|
expected = {
|
||||||
'OptionSet-Once',
|
'OptionSet-Once',
|
||||||
@ -250,6 +250,24 @@ describe('autocmd', function()
|
|||||||
--- Autocommands ---]]),
|
--- Autocommands ---]]),
|
||||||
fn.execute('autocmd Tabnew')
|
fn.execute('autocmd Tabnew')
|
||||||
)
|
)
|
||||||
|
|
||||||
|
--
|
||||||
|
-- :autocmd does not recursively call ++once Lua handlers.
|
||||||
|
--
|
||||||
|
exec_lua [[vim.g.count = 0]]
|
||||||
|
eq(0, eval('g:count'))
|
||||||
|
exec_lua [[
|
||||||
|
vim.api.nvim_create_autocmd('User', {
|
||||||
|
once = true,
|
||||||
|
pattern = nil,
|
||||||
|
callback = function()
|
||||||
|
vim.g.count = vim.g.count + 1
|
||||||
|
vim.api.nvim_exec_autocmds('User', { pattern = nil })
|
||||||
|
end,
|
||||||
|
})
|
||||||
|
vim.api.nvim_exec_autocmds('User', { pattern = nil })
|
||||||
|
]]
|
||||||
|
eq(1, eval('g:count'))
|
||||||
end)
|
end)
|
||||||
|
|
||||||
it('internal `aucmd_win` window', function()
|
it('internal `aucmd_win` window', function()
|
||||||
|
Reference in New Issue
Block a user