From df21ac729cf69fdca44af1874572f337bfc3a45a Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 28 Jan 2026 06:21:28 +0800 Subject: [PATCH] fix(terminal): losing output if BufFile* poll for events (#37580) Problem: Terminal loses output if a BufFilePre or BufFilePost autocmd polls for events. Solution: Rename the buffer after allocating the terminal instance. Also fix buffer getting wrong name if BufFilePre uses NameBuff. --- src/nvim/api/vim.c | 3 +- src/nvim/channel.c | 6 +- src/nvim/eval/funcs.c | 37 ++++++--- src/nvim/terminal.c | 24 +++++- test/functional/terminal/buffer_spec.lua | 27 ++++++- test/functional/terminal/channel_spec.lua | 99 +++++++++++++++-------- 6 files changed, 141 insertions(+), 55 deletions(-) diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index 8eadb63e76..f5f8ade7e9 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -1155,7 +1155,8 @@ Integer nvim_open_term(Buffer buffer, Dict(open_term) *opts, Error *err) } channel_incref(chan); - terminal_open(&chan->term, buf, topts); + chan->term = terminal_alloc(buf, topts); + terminal_open(&chan->term, buf); if (chan->term != NULL) { terminal_check_size(chan->term); } diff --git a/src/nvim/channel.c b/src/nvim/channel.c index e417e14d5b..12bd9a2573 100644 --- a/src/nvim/channel.c +++ b/src/nvim/channel.c @@ -813,11 +813,11 @@ static void channel_callback_call(Channel *chan, CallbackReader *reader) } } -/// Open terminal for channel +/// Allocate terminal for channel /// /// Channel `chan` is assumed to be an open pty channel, /// and `buf` is assumed to be a new, unmodified buffer. -void channel_terminal_open(buf_T *buf, Channel *chan) +void channel_terminal_alloc(buf_T *buf, Channel *chan) { TerminalOptions topts = { .data = chan, @@ -830,7 +830,7 @@ void channel_terminal_open(buf_T *buf, Channel *chan) }; buf->b_p_channel = (OptInt)chan->id; // 'channel' option channel_incref(chan); - terminal_open(&chan->term, buf, topts); + chan->term = terminal_alloc(buf, topts); } static void term_write(const char *buf, size_t size, void *data) diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 871794c4a1..8bea81d752 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -3603,7 +3603,17 @@ void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) return; } - int pid = chan->stream.pty.proc.pid; + const int pid = chan->stream.pty.proc.pid; + buf_T *const buf = curbuf; + + channel_incref(chan); + channel_terminal_alloc(buf, chan); + + apply_autocmds(EVENT_BUFFILEPRE, NULL, NULL, false, buf); + + if (chan->term == NULL || terminal_buf(chan->term) == 0) { + goto term_done; // Terminal may be destroyed during autocommands. + } // "./…" => "/home/foo/…" vim_FullName(cwd, NameBuff, sizeof(NameBuff), false); @@ -3623,23 +3633,32 @@ void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) // Terminal URI: "term://$CWD//$PID:$CMD" snprintf(NameBuff, sizeof(NameBuff), "term://%s//%d:%s", IObuff, pid, cmd); // Buffer has no terminal associated yet; unset 'swapfile' to ensure no swapfile is created. - curbuf->b_p_swf = false; + buf->b_p_swf = false; - apply_autocmds(EVENT_BUFFILEPRE, NULL, NULL, false, curbuf); - setfname(curbuf, NameBuff, NULL, true); - apply_autocmds(EVENT_BUFFILEPOST, NULL, NULL, false, curbuf); + setfname(buf, NameBuff, NULL, true); + apply_autocmds(EVENT_BUFFILEPOST, NULL, NULL, false, buf); + + if (chan->term == NULL || terminal_buf(chan->term) == 0) { + goto term_done; // Terminal may be destroyed during autocommands. + } Error err = ERROR_INIT; + buf->b_locked++; // Set (deprecated) buffer-local vars (prefer 'channel' buffer-local option). - dict_set_var(curbuf->b_vars, cstr_as_string("terminal_job_id"), + dict_set_var(buf->b_vars, cstr_as_string("terminal_job_id"), INTEGER_OBJ((Integer)chan->id), false, false, NULL, &err); api_clear_error(&err); - dict_set_var(curbuf->b_vars, cstr_as_string("terminal_job_pid"), + dict_set_var(buf->b_vars, cstr_as_string("terminal_job_pid"), INTEGER_OBJ(pid), false, false, NULL, &err); api_clear_error(&err); + buf->b_locked--; - channel_incref(chan); - channel_terminal_open(curbuf, chan); + if (chan->term == NULL || terminal_buf(chan->term) == 0) { + goto term_done; // Terminal may be destroyed in dict watchers. + } + + terminal_open(&chan->term, buf); +term_done: channel_create_event(chan, NULL); channel_decref(chan); } diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 92019bb513..6fa495f062 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -143,7 +143,7 @@ typedef struct { } ScrollbackLine; struct terminal { - TerminalOptions opts; // options passed to terminal_open + TerminalOptions opts; // options passed to terminal_alloc() VTerm *vt; VTermScreen *vts; // buffer used to: @@ -474,18 +474,20 @@ static bool term_may_alloc_scrollback(Terminal *term, buf_T *buf) // public API {{{ -/// Initializes terminal properties, and triggers TermOpen. +/// Allocates a terminal instance and initializes terminal properties. /// /// The PTY process (TerminalOptions.data) was already started by jobstart(), /// via ex_terminal() or the term:// BufReadCmd. /// /// @param buf Buffer used for presentation of the terminal. /// @param opts PTY process channel, various terminal properties and callbacks. -void terminal_open(Terminal **termpp, buf_T *buf, TerminalOptions opts) +/// +/// @return the terminal instance. +Terminal *terminal_alloc(buf_T *buf, TerminalOptions opts) FUNC_ATTR_NONNULL_ALL { // Create a new terminal instance and configure it - Terminal *term = *termpp = xcalloc(1, sizeof(Terminal)); + Terminal *term = xcalloc(1, sizeof(Terminal)); term->opts = opts; // Associate the terminal instance with the new buffer @@ -545,6 +547,19 @@ void terminal_open(Terminal **termpp, buf_T *buf, TerminalOptions opts) // events from this queue are copied back onto the main event queue. term->pending.events = multiqueue_new(NULL, NULL); + return term; +} + +/// Triggers TermOpen and allocates terminal scrollback buffer. +/// +/// @param termpp Pointer to the terminal channel's `term` field. +/// @param buf Buffer used for presentation of the terminal. +void terminal_open(Terminal **termpp, buf_T *buf) + FUNC_ATTR_NONNULL_ALL +{ + Terminal *term = *termpp; + assert(term != NULL); + aco_save_T aco; aucmd_prepbuf(&aco, buf); @@ -575,6 +590,7 @@ void terminal_open(Terminal **termpp, buf_T *buf, TerminalOptions opts) abort(); } + VTermState *state = vterm_obtain_state(term->vt); // Configure the color palette. Try to get the color from: // // - b:terminal_color_{NUM} diff --git a/test/functional/terminal/buffer_spec.lua b/test/functional/terminal/buffer_spec.lua index a8cc6bdb40..6f65fdc77c 100644 --- a/test/functional/terminal/buffer_spec.lua +++ b/test/functional/terminal/buffer_spec.lua @@ -801,10 +801,9 @@ describe(':terminal buffer', function() check_term_rep_20000('REPFAST') end) - -- it('does not drop data when autocommands poll for events #37559', function() - it('does not drop data when TermOpen polls for events', function() - -- api.nvim_create_autocmd('BufFilePre', { command = 'sleep 50m', nested = true }) - -- api.nvim_create_autocmd('BufFilePost', { command = 'sleep 50m', nested = true }) + it('does not drop data when autocommands poll for events #37559', function() + api.nvim_create_autocmd('BufFilePre', { command = 'sleep 50m', nested = true }) + api.nvim_create_autocmd('BufFilePost', { command = 'sleep 50m', nested = true }) api.nvim_create_autocmd('TermOpen', { command = 'sleep 50m', nested = true }) -- REP pauses 1 ms every 100 lines, so each autocommand processes some output. check_term_rep_20000('REP') @@ -1144,6 +1143,26 @@ describe(':terminal buffer', function() eq('OTHER_TITLE', api.nvim_buf_get_var(0, 'term_title')) matches('^E937: ', api.nvim_get_vvar('errmsg')) end) + + it('using NameBuff in BufFilePre does not interfere with buffer rename', function() + local oldbuf = api.nvim_get_current_buf() + n.exec([[ + file Xoldfile + new Xotherfile + wincmd w + let g:BufFilePre_bufs = [] + let g:BufFilePost_bufs = [] + autocmd BufFilePre * call add(g:BufFilePre_bufs, [bufnr(), bufname()]) + autocmd BufFilePost * call add(g:BufFilePost_bufs, [bufnr(), bufname()]) + autocmd BufFilePre,BufFilePost * call execute('ls') + ]]) + fn.jobstart({ testprg('shell-test') }, { term = true }) + eq({ { oldbuf, 'Xoldfile' } }, api.nvim_get_var('BufFilePre_bufs')) + local buffilepost_bufs = api.nvim_get_var('BufFilePost_bufs') + eq(1, #buffilepost_bufs) + eq(oldbuf, buffilepost_bufs[1][1]) + matches('^term://', buffilepost_bufs[1][2]) + end) end) describe('on_lines does not emit out-of-bounds line indexes when', function() diff --git a/test/functional/terminal/channel_spec.lua b/test/functional/terminal/channel_spec.lua index 924909f2d2..7f93096e3f 100644 --- a/test/functional/terminal/channel_spec.lua +++ b/test/functional/terminal/channel_spec.lua @@ -138,41 +138,53 @@ it('chansend sends lines to terminal channel in proper order', function() end end) -describe('no crash when TermOpen autocommand', function() - local screen +--- @param event string +--- @param extra_tests fun(table, table)? +local function test_autocmd_no_crash(event, extra_tests) + local env = {} -- Use REPFAST for immediately output after start. local term_args = { testprg('shell-test'), 'REPFAST', '50', 'TEST' } before_each(function() clear() - screen = Screen.new(60, 4) - command([[call setline(1, 'OLDBUF') | enew]]) + env.screen = Screen.new(60, 4) + command([[file Xoldbuf | call setline(1, 'OLDBUF') | enew]]) + -- Wait before :bwipe to avoid closing PTY master before the child calls setsid(), + -- as that will cause SIGHUP to be also sent to the parent. + -- Use vim.uv.sleep() which blocks the event loop. + n.exec([[ + func Wipe() + lua vim.uv.sleep(5) + bwipe! + endfunc + ]]) end) it('processes job exit event when using jobstart(…,{term=true})', function() - command([[autocmd TermOpen * call input('')]]) + api.nvim_create_autocmd(event, { command = "call input('')" }) async_meths.nvim_call_function('jobstart', { term_args, { term = true } }) - screen:expect([[ + env.screen:expect([[ | {1:~ }|*2 ^ | ]]) + vim.uv.sleep(20) feed('') - screen:expect([[ + env.screen:expect([[ ^0: TEST | 1: TEST | 2: TEST | | ]]) feed('i') - screen:expect([[ + env.screen:expect([[ 49: TEST | | [Process exited 0]^ | {5:-- TERMINAL --} | ]]) feed('') - screen:expect([[ + env.screen:expect([[ ^OLDBUF | {1:~ }|*2 | @@ -181,50 +193,69 @@ describe('no crash when TermOpen autocommand', function() end) it('wipes buffer and processes events when using jobstart(…,{term=true})', function() - command([[autocmd TermOpen * bwipe! | call input('')]]) + api.nvim_create_autocmd(event, { command = "call Wipe() | call input('')" }) async_meths.nvim_call_function('jobstart', { term_args, { term = true } }) - screen:expect([[ + env.screen:expect([[ | {1:~ }|*2 ^ | ]]) + vim.uv.sleep(20) feed('') - screen:expect([[ + env.screen:expect([[ ^OLDBUF | {1:~ }|*2 | ]]) assert_alive() + eq('Xoldbuf', eval('bufname()')) + eq(0, eval([[exists('b:term_title')]])) end) - it('wipes buffer and processes events when using nvim_open_term()', function() - command([[autocmd TermOpen * bwipe! | call input('')]]) - async_meths.nvim_open_term(0, {}) - screen:expect([[ - | - {1:~ }|*2 - ^ | - ]]) - feed('') - screen:expect([[ - ^OLDBUF | - {1:~ }|*2 - | - ]]) - assert_alive() - end) + if extra_tests then + extra_tests(env, term_args) + end +end - it('wipes buffer when using jobstart(…,{term=true}) during Nvim exit', function() - n.expect_exit(n.exec_lua, function() - vim.schedule(function() - vim.fn.jobstart(term_args, { term = true }) +describe('no crash when TermOpen autocommand', function() + test_autocmd_no_crash('TermOpen', function(env, term_args) + it('wipes buffer and processes events when using nvim_open_term()', function() + api.nvim_create_autocmd('TermOpen', { command = "call Wipe() | call input('')" }) + async_meths.nvim_open_term(0, {}) + env.screen:expect([[ + | + {1:~ }|*2 + ^ | + ]]) + feed('') + env.screen:expect([[ + ^OLDBUF | + {1:~ }|*2 + | + ]]) + assert_alive() + end) + + it('wipes buffer when using jobstart(…,{term=true}) during Nvim exit', function() + n.expect_exit(n.exec_lua, function() + vim.schedule(function() + vim.fn.jobstart(term_args, { term = true }) + end) + vim.api.nvim_create_autocmd('TermOpen', { command = 'call Wipe()' }) + vim.cmd('qall!') end) - vim.cmd('autocmd TermOpen * bwipe!') - vim.cmd('qall!') end) end) end) +describe('no crash when BufFilePre autocommand', function() + test_autocmd_no_crash('BufFilePre') +end) + +describe('no crash when BufFilePost autocommand', function() + test_autocmd_no_crash('BufFilePost') +end) + describe('nvim_open_term', function() local screen