diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index 9e99db8d32..b1e05186e0 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -1018,6 +1018,15 @@ Integer nvim_open_term(Buffer buffer, Dict(open_term) *opts, Error *err) return 0; } + if (buf->terminal) { + if (terminal_running(buf->terminal)) { + api_set_error(err, kErrorTypeException, + "Terminal already connected to buffer %d", buf->handle); + return 0; + } + buf_close_terminal(buf); + } + LuaRef cb = LUA_NOREF; if (HAS_KEY(opts, open_term, on_input)) { cb = opts->on_input; diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index 4d46a3ec7c..8dba76bf01 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -480,6 +480,14 @@ static bool can_unload_buffer(buf_T *buf) return can_unload; } +void buf_close_terminal(buf_T *buf) +{ + assert(buf->terminal); + buf->b_locked++; + terminal_close(&buf->terminal, -1); + buf->b_locked--; +} + /// Close the link to a buffer. /// /// @param win If not NULL, set b_last_cursor. @@ -629,9 +637,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i } if (buf->terminal) { - buf->b_locked++; - terminal_close(&buf->terminal, -1); - buf->b_locked--; + buf_close_terminal(buf); } // Always remove the buffer when there is no file name. diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 8451cb78b2..752b367f63 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -126,6 +126,7 @@ #include "nvim/strings.h" #include "nvim/syntax.h" #include "nvim/tag.h" +#include "nvim/terminal.h" #include "nvim/types_defs.h" #include "nvim/ui.h" #include "nvim/ui_compositor.h" @@ -4061,6 +4062,14 @@ void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) shell_free_argv(argv); return; } + if (curbuf->terminal) { + if (terminal_running(curbuf->terminal)) { + semsg(_("Terminal already connected to buffer %d"), curbuf->handle); + shell_free_argv(argv); + return; + } + buf_close_terminal(curbuf); + } assert(!rpc); term_name = "xterm-256color"; cwd = cwd ? cwd : "."; diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 89a1068387..c407176142 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -581,7 +581,7 @@ void terminal_close(Terminal **termpp, int status) #ifdef EXITFREE if (entered_free_all_mem) { - // If called from close_buffer() inside free_all_mem(), the main loop has + // If called from buf_close_terminal() inside free_all_mem(), the main loop has // already been freed, so it is not safe to call the close callback here. terminal_destroy(termpp); return; @@ -591,7 +591,7 @@ void terminal_close(Terminal **termpp, int status) bool only_destroy = false; if (term->closed) { - // If called from close_buffer() after the process has already exited, we + // If called from buf_close_terminal() after the process has already exited, we // only need to call the close callback to clean up the terminal object. only_destroy = true; } else { @@ -608,9 +608,9 @@ void terminal_close(Terminal **termpp, int status) buf_T *buf = handle_get_buffer(term->buf_handle); if (status == -1 || exiting) { - // If this was called by close_buffer() (status is -1), or if exiting, we - // must inform the buffer the terminal no longer exists so that - // close_buffer() won't call this again. + // If this was called by buf_close_terminal() (status is -1), or if exiting, we + // must inform the buffer the terminal no longer exists so that close_buffer() + // won't call buf_close_terminal() again. // If inside Terminal mode event handling, setting buf_handle to 0 also // informs terminal_enter() to call the close callback before returning. term->buf_handle = 0; @@ -871,7 +871,7 @@ static bool terminal_check_focus(TerminalState *const s) set_terminal_winopts(s); } if (s->term != curbuf->terminal) { - // Active terminal buffer changed, flush terminal's cursor state to the UI. + // Active terminal changed, flush terminal's cursor state to the UI. terminal_focus(s->term, false); if (s->close) { s->term->destroy = true; diff --git a/test/functional/terminal/buffer_spec.lua b/test/functional/terminal/buffer_spec.lua index 48b2fcc601..a0ddfa435a 100644 --- a/test/functional/terminal/buffer_spec.lua +++ b/test/functional/terminal/buffer_spec.lua @@ -882,6 +882,148 @@ describe(':terminal buffer', function() ]]) eq(false, api.nvim_buf_is_valid(term_buf)) end) + + local function test_open_term_in_buf_with_running_term(env) + describe('does not allow', function() + it('opening another terminal with jobstart() in same buffer', function() + eq( + ('Vim:Terminal already connected to buffer %d'):format(env.buf), + pcall_err(fn.jobstart, { testprg('tty-test') }, { term = true }) + ) + env.screen:expect_unchanged() + end) + + it('opening another terminal with nvim_open_term() in same buffer', function() + eq( + ('Terminal already connected to buffer %d'):format(env.buf), + pcall_err(api.nvim_open_term, env.buf, {}) + ) + env.screen:expect_unchanged() + end) + end) + end + + describe('with running terminal job', function() + local env = {} + + before_each(function() + env.screen = Screen.new(60, 6) + fn.jobstart({ testprg('tty-test') }, { term = true }) + env.screen:expect([[ + ^tty ready | + |*5 + ]]) + env.buf = api.nvim_get_current_buf() + api.nvim_set_option_value('modified', false, { buf = env.buf }) + end) + + test_open_term_in_buf_with_running_term(env) + end) + + describe('with open nvim_open_term() channel', function() + local env = {} + + before_each(function() + env.screen = Screen.new(60, 6) + local chan = api.nvim_open_term(0, {}) + api.nvim_chan_send(chan, 'TEST') + env.screen:expect([[ + ^TEST | + |*5 + ]]) + env.buf = api.nvim_get_current_buf() + api.nvim_set_option_value('modified', false, { buf = env.buf }) + end) + + test_open_term_in_buf_with_running_term(env) + end) + + local function test_open_term_in_buf_with_closed_term(env) + describe('does not leak memory when', function() + describe('opening another terminal with jobstart() in same buffer', function() + it('in Normal mode', function() + fn.jobstart({ testprg('tty-test') }, { term = true }) + env.screen:expect([[ + ^tty ready | + |*5 + ]]) + end) + + it('in Terminal mode', function() + feed('i') + eq({ blocking = false, mode = 't' }, api.nvim_get_mode()) + fn.jobstart({ testprg('tty-test') }, { term = true }) + env.screen:expect([[ + tty ready | + ^ | + |*3 + {5:-- TERMINAL --} | + ]]) + end) + end) + + describe('opening another terminal with nvim_open_term() in same buffer', function() + it('in Normal mode', function() + local chan = api.nvim_open_term(env.buf, {}) + api.nvim_chan_send(chan, 'OTHER') + env.screen:expect([[ + ^OTHER | + |*5 + ]]) + end) + + it('in Terminal mode', function() + feed('i') + eq({ blocking = false, mode = 't' }, api.nvim_get_mode()) + local chan = api.nvim_open_term(env.buf, {}) + api.nvim_chan_send(chan, 'OTHER') + env.screen:expect([[ + OTHER^ | + |*4 + {5:-- TERMINAL --} | + ]]) + end) + end) + end) + end + + describe('with exited terminal job', function() + local env = {} + + before_each(function() + env.screen = Screen.new(60, 6) + fn.jobstart({ testprg('shell-test') }, { term = true }) + env.screen:expect([[ + ^ready $ | + [Process exited 0] | + |*4 + ]]) + env.buf = api.nvim_get_current_buf() + api.nvim_set_option_value('modified', false, { buf = env.buf }) + end) + + test_open_term_in_buf_with_closed_term(env) + end) + + describe('with closed nvim_open_term() channel', function() + local env = {} + + before_each(function() + env.screen = Screen.new(60, 6) + local chan = api.nvim_open_term(0, {}) + api.nvim_chan_send(chan, 'TEST') + fn.chanclose(chan) + env.screen:expect([[ + ^TEST | + [Terminal closed] | + |*4 + ]]) + env.buf = api.nvim_get_current_buf() + api.nvim_set_option_value('modified', false, { buf = env.buf }) + end) + + test_open_term_in_buf_with_closed_term(env) + end) end) describe('on_lines does not emit out-of-bounds line indexes when', function()