commit 228ff6e5476e0b73320d32493a5d85da63836e3c
parent b98eefd8030c398735f4069ddc05b3e5b7a511a5
Author: zeertzjq <zeertzjq@outlook.com>
Date: Tue, 10 Jun 2025 23:27:14 +0800
Merge pull request #34411 from zeertzjq/tui-crash
fix(tui): wait for embedded server's exit code
Diffstat:
10 files changed, 141 insertions(+), 55 deletions(-)
diff --git a/runtime/doc/gui.txt b/runtime/doc/gui.txt
@@ -81,7 +81,6 @@ Restart Nvim
Note: If the current UI hasn't implemented the "restart" UI
event, this command is equivalent to `:qall`.
Note: Only works if the UI and server are on the same system.
- Note: Not supported on Windows yet.
:restart!
Force restarts the Nvim server, abandoning unsaved buffers.
diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
@@ -73,12 +73,26 @@ static void remote_ui_destroy(RemoteUI *ui)
xfree(ui);
}
-void remote_ui_disconnect(uint64_t channel_id)
+/// Removes the client on the given channel from the list of UIs.
+///
+/// @param err if non-NULL and there is no UI on the channel, set an error
+/// @param send_error_exit send an "error_exit" event with 0 status first
+void remote_ui_disconnect(uint64_t channel_id, Error *err, bool send_error_exit)
{
RemoteUI *ui = pmap_get(uint64_t)(&connected_uis, channel_id);
if (!ui) {
+ if (err != NULL) {
+ api_set_error(err, kErrorTypeException,
+ "UI not attached to channel: %" PRId64, channel_id);
+ }
return;
}
+ if (send_error_exit) {
+ MAXSIZE_TEMP_ARRAY(args, 1);
+ ADD_C(args, INTEGER_OBJ(0));
+ push_call(ui, "error_exit", args);
+ ui_flush_buf(ui);
+ }
pmap_del(uint64_t)(&connected_uis, channel_id, NULL);
ui_detach_impl(ui, channel_id);
remote_ui_destroy(ui);
@@ -233,12 +247,7 @@ void nvim_ui_set_focus(uint64_t channel_id, Boolean gained, Error *error)
void nvim_ui_detach(uint64_t channel_id, Error *err)
FUNC_API_SINCE(1) FUNC_API_REMOTE_ONLY
{
- if (!map_has(uint64_t, &connected_uis, channel_id)) {
- api_set_error(err, kErrorTypeException,
- "UI not attached to channel: %" PRId64, channel_id);
- return;
- }
- remote_ui_disconnect(channel_id);
+ remote_ui_disconnect(channel_id, err, false);
}
/// Sends a "restart" UI event to the UI on the given channel.
diff --git a/src/nvim/api/ui_events.in.h b/src/nvim/api/ui_events.in.h
@@ -177,5 +177,13 @@ void msg_history_show(Array entries)
void msg_history_clear(void)
FUNC_API_SINCE(10) FUNC_API_REMOTE_ONLY;
+// This UI event is currently undocumented.
+// - When the server needs to intentionally exit with an exit code, and there is no
+// message in server stderr for the user, this event is sent with positive `status`
+// argument, to indicate that the UI should exit normally with `status`.
+// - When the server has crashed or there is a message in server stderr for the user,
+// this event is not sent, and the UI should make server stderr visible.
+// - When :detach is used on the server, this event is sent with a zero `status`
+// argument, to indicate that the UI shouldn't wait for server exit.
void error_exit(Integer status)
- FUNC_API_SINCE(12);
+ FUNC_API_SINCE(12) FUNC_API_CLIENT_IMPL;
diff --git a/src/nvim/event/proc.c b/src/nvim/event/proc.c
@@ -5,6 +5,7 @@
#include <uv.h>
#include "klib/kvec.h"
+#include "nvim/channel.h"
#include "nvim/event/libuv_proc.h"
#include "nvim/event/loop.h"
#include "nvim/event/multiqueue.h"
@@ -437,16 +438,25 @@ static void on_proc_exit(Proc *proc)
Loop *loop = proc->loop;
ILOG("child exited: pid=%d status=%d" PRIu64, proc->pid, proc->status);
-#ifdef MSWIN
- // XXX: This assumes the TUI never spawns any other processes...?
- // TODO(justinmk): figure out why rpc_close sometimes(??) isn't called, then remove this jank.
+ // TODO(justinmk): figure out why rpc_close sometimes(??) isn't called.
// Theories:
// - EOF not received in receive_msgpack, then doesn't call chan_close_on_err().
// - proc_close_handles not tickled by ui_client.c's LOOP_PROCESS_EVENTS?
if (ui_client_channel_id) {
- exit_on_closed_chan(proc->status);
+ uint64_t server_chan_id = ui_client_channel_id;
+ Channel *server_chan = find_channel(server_chan_id);
+ if (server_chan != NULL && server_chan->streamtype == kChannelStreamProc
+ && proc == &server_chan->stream.proc) {
+ // Need to call ui_client_may_restart_server() here as well, as sometimes
+ // rpc_close_event() hasn't been called yet (also see comments above).
+ ui_client_may_restart_server();
+ if (ui_client_channel_id == server_chan_id) {
+ // If the current embedded server has exited and no new server is started,
+ // the client should exit with the same status.
+ exit_on_closed_chan(proc->status);
+ }
+ }
}
-#endif
// Process has terminated, but there could still be data to be read from the
// OS. We are still in the libuv loop, so we cannot call code that polls for
diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c
@@ -5560,8 +5560,8 @@ static void ex_detach(exarg_T *eap)
if (eap && eap->forceit) {
emsg("bang (!) not supported yet");
} else {
- // 1. (TODO) Send "detach" UI-event (notification only).
- // 2. Perform server-side `nvim_ui_detach`.
+ // 1. Send "error_exit" UI-event (notification only).
+ // 2. Perform server-side UI detach.
// 3. Close server-side channel without self-exit.
if (!current_ui) {
@@ -5578,7 +5578,7 @@ static void ex_detach(exarg_T *eap)
// Server-side UI detach. Doesn't close the channel.
Error err2 = ERROR_INIT;
- nvim_ui_detach(chan->id, &err2);
+ remote_ui_disconnect(chan->id, &err2, true);
if (ERROR_SET(&err2)) {
emsg(err2.msg); // UI disappeared already?
api_clear_error(&err2);
diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c
@@ -498,7 +498,7 @@ static void rpc_close_event(void **argv)
if (is_ui_client || channel->streamtype == kChannelStreamStdio) {
if (!is_ui_client) {
// Avoid hanging when there are no other UIs and a prompt is triggered on exit.
- remote_ui_disconnect(channel->id);
+ remote_ui_disconnect(channel->id, NULL, false);
} else {
ui_client_may_restart_server();
if (ui_client_channel_id != channel->id) {
@@ -507,14 +507,19 @@ static void rpc_close_event(void **argv)
}
}
if (!channel->detach) {
- exit_on_closed_chan(channel->exit_status == -1 ? 0 : channel->exit_status);
+ if (channel->streamtype == kChannelStreamProc && ui_client_error_exit < 0) {
+ // Wait for the embedded server to exit instead of exiting immediately,
+ // as it's necessary to get the server's exit code in on_proc_exit().
+ } else {
+ exit_on_closed_chan(0);
+ }
}
}
}
void rpc_free(Channel *channel)
{
- remote_ui_disconnect(channel->id);
+ remote_ui_disconnect(channel->id, NULL, false);
unpacker_teardown(channel->rpc.unpacker);
xfree(channel->rpc.unpacker);
diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
@@ -142,7 +142,6 @@ struct TUIData {
char *space_buf;
size_t space_buf_len;
bool stopped;
- int seen_error_exit;
int width;
int height;
bool rgb;
@@ -165,7 +164,6 @@ void tui_start(TUIData **tui_p, int *width, int *height, char **term, bool *rgb)
tui->is_starting = true;
tui->screenshot = NULL;
tui->stopped = false;
- tui->seen_error_exit = 0;
tui->loop = &main_loop;
tui->url = -1;
@@ -568,14 +566,16 @@ static void terminfo_disable(TUIData *tui)
/// Disable the alternate screen and prepare for the TUI to close.
static void terminfo_stop(TUIData *tui)
{
- if (ui_client_exit_status == 0) {
- ui_client_exit_status = tui->seen_error_exit;
+ if (ui_client_exit_status == 0 && ui_client_error_exit > 0) {
+ ui_client_exit_status = ui_client_error_exit;
}
- // if nvim exited with nonzero status, without indicated this was an
+ // If Nvim exited with nonzero status, without indicating this was an
// intentional exit (like `:1cquit`), it likely was an internal failure.
- // Don't clobber the stderr error message in this case.
- if (ui_client_exit_status == tui->seen_error_exit) {
+ // Don't clobber the stderr error message in this case. #21608
+ if (ui_client_exit_status == MAX(ui_client_error_exit, 0)) {
+ // Position the cursor on the last screen line, below all the text
+ cursor_goto(tui, tui->height - 1, 0);
// Exit alternate screen.
unibi_out(tui, unibi_exit_ca_mode);
}
@@ -614,12 +614,6 @@ static void tui_terminal_after_startup(TUIData *tui)
flush_buf(tui);
}
-void tui_error_exit(TUIData *tui, Integer status)
- FUNC_ATTR_NONNULL_ALL
-{
- tui->seen_error_exit = (int)status;
-}
-
void tui_stop(TUIData *tui)
FUNC_ATTR_NONNULL_ALL
{
@@ -660,8 +654,6 @@ static void tui_terminal_stop(TUIData *tui)
FUNC_ATTR_NONNULL_ALL
{
tinput_stop(&tui->input);
- // Position the cursor on the last screen line, below all the text
- cursor_goto(tui, tui->height - 1, 0);
terminfo_stop(tui);
}
diff --git a/src/nvim/ui_client.c b/src/nvim/ui_client.c
@@ -348,6 +348,16 @@ cleanup:
restart_args = (Array)ARRAY_DICT_INIT;
}
+void ui_client_event_error_exit(Array args)
+{
+ if (args.size < 1
+ || args.items[0].type != kObjectTypeInteger) {
+ ELOG("Error handling ui event 'error_exit'");
+ return;
+ }
+ ui_client_error_exit = (int)args.items[0].data.integer;
+}
+
#ifdef EXITFREE
void ui_client_free_all_mem(void)
{
diff --git a/src/nvim/ui_client.h b/src/nvim/ui_client.h
@@ -17,7 +17,11 @@ EXTERN sattr_T *grid_line_buf_attr INIT( = NULL);
// Client-side UI channel. Zero during early startup or if not a (--remote-ui) UI client.
EXTERN uint64_t ui_client_channel_id INIT( = 0);
-// exit status from embedded nvim process
+/// `status` argument of the last "error_exit" UI event, or -1 if none has been seen.
+/// NOTE: This assumes "error_exit" never has a negative `status` argument.
+EXTERN int ui_client_error_exit INIT( = -1);
+
+/// Server exit code.
EXTERN int ui_client_exit_status INIT( = 0);
/// Whether ui client has sent nvim_ui_attach yet
diff --git a/test/functional/terminal/tui_spec.lua b/test/functional/terminal/tui_spec.lua
@@ -33,6 +33,28 @@ local assert_log = t.assert_log
local testlog = 'Xtest-tui-log'
+describe('TUI', function()
+ it('exit status 1 and error message with server --listen error #34365', function()
+ clear()
+ local addr_in_use = api.nvim_get_vvar('servername')
+ local screen = tt.setup_child_nvim(
+ { '--listen', addr_in_use, '-u', 'NONE', '-i', 'NONE' },
+ { extra_rows = 10, cols = 60 }
+ )
+ -- When the address is very long, the error message may be only partly visible.
+ if #addr_in_use <= 600 then
+ screen:expect({
+ any = vim.pesc(
+ ('%s: Failed to --listen: address already in use:'):format(
+ is_os('win') and 'nvim.exe' or 'nvim'
+ )
+ ),
+ })
+ end
+ screen:expect({ any = vim.pesc('[Process exited 1]') })
+ end)
+end)
+
describe('TUI :detach', function()
it('does not stop server', function()
local job_opts = { env = {} }
@@ -157,10 +179,6 @@ describe('TUI :detach', function()
end)
end)
-if t.skip(is_os('win')) then
- return
-end
-
describe('TUI :restart', function()
it('resets buffer to blank', function()
clear()
@@ -184,6 +202,19 @@ describe('TUI :restart', function()
'echo getpid()',
})
+ --- FIXME: On Windows spaces at the end of a screen line may have wrong attrs.
+ --- Remove this function when that's fixed.
+ ---
+ --- @param s string
+ local function screen_expect(s)
+ if is_os('win') then
+ s = s:gsub(' +%}%|\n', '{MATCH: *}}{MATCH: *}|\n')
+ s = s:gsub(' *%} +%|\n', '{MATCH: *}}{MATCH: *}|\n')
+ s = s:gsub('%}%^ +%|\n', '{MATCH:[ ^]*}}{MATCH:[ ^]*}|\n')
+ end
+ screen:expect(s)
+ end
+
local s0 = [[
^ |
{4:~ }|*3
@@ -191,11 +222,20 @@ describe('TUI :restart', function()
{MATCH:%d+ +}|
{3:-- TERMINAL --} |
]]
- screen:expect(s0)
- local server_session = n.connect(server_pipe)
- local _, server_pid = server_session:request('nvim_call_function', 'getpid', {})
+ screen_expect(s0)
+
+ local server_session --[[@type test.Session]]
+ local server_pid --[[@type any]]
+ -- FIXME: On Windows connect() hangs.
+ if not is_os('win') then
+ server_session = n.connect(server_pipe)
+ _, server_pid = server_session:request('nvim_call_function', 'getpid', {})
+ end
local function restart_pid_check()
+ if is_os('win') then
+ return
+ end
server_session:close()
server_session = n.connect(server_pipe)
local _, new_pid = server_session:request('nvim_call_function', 'getpid', {})
@@ -211,11 +251,11 @@ describe('TUI :restart', function()
-- Check ":restart" on an unmodified buffer.
tt.feed_data(':restart\013')
- screen:expect(s0)
+ screen_expect(s0)
restart_pid_check()
tt.feed_data('ithis will be removed\027')
- screen:expect([[
+ screen_expect([[
this will be remove^d |
{4:~ }|*3
{5:[No Name] [+] }|
@@ -225,7 +265,7 @@ describe('TUI :restart', function()
-- Check ":restart" on a modified buffer.
tt.feed_data(':restart\013')
- screen:expect([[
+ screen_expect([[
this will be removed |
{5: }|
{8:E37: No write since last change} |
@@ -237,11 +277,11 @@ describe('TUI :restart', function()
-- Check ":restart!".
tt.feed_data(':restart!\013')
- screen:expect(s0)
+ screen_expect(s0)
restart_pid_check()
tt.feed_data(':echo\n')
- screen:expect([[
+ screen_expect([[
^ |
{4:~ }|*3
{5:[No Name] }|
@@ -251,11 +291,11 @@ describe('TUI :restart', function()
-- No --listen conflict when server exit is delayed.
feed_data(':lua vim.schedule(function() vim.wait(100) end); vim.cmd.restart()\n')
- screen:expect(s0)
+ screen_expect(s0)
restart_pid_check()
screen:try_resize(60, 6)
- screen:expect([[
+ screen_expect([[
^ |
{4:~ }|*2
{5:[No Name] }|
@@ -265,7 +305,7 @@ describe('TUI :restart', function()
--- Check that ":restart" uses the updated size after terminal resize.
tt.feed_data(':restart\013')
- screen:expect([[
+ screen_expect([[
^ |
{4:~ }|*2
{5:[No Name] }|
@@ -276,6 +316,10 @@ describe('TUI :restart', function()
end)
end)
+if t.skip(is_os('win')) then
+ return
+end
+
describe('TUI', function()
local screen --[[@type test.functional.ui.screen]]
local child_session --[[@type test.Session]]
@@ -2227,15 +2271,20 @@ describe('TUI', function()
it('no assert failure on deadly signal #21896', function()
exec_lua([[vim.uv.kill(vim.fn.jobpid(vim.bo.channel), 'sigterm')]])
- screen:expect {
- grid = [[
+ screen:expect([[
Nvim: Caught deadly signal 'SIGTERM' |
|
[Process exited 1]^ |
|*3
{3:-- TERMINAL --} |
- ]],
- }
+ ]])
+ end)
+
+ it('exit status 1 and error message with deadly signal sent to server', function()
+ local _, server_pid = child_session:request('nvim_call_function', 'getpid', {})
+ exec_lua([[vim.uv.kill(..., 'sigterm')]], server_pid)
+ screen:expect({ any = vim.pesc([[Nvim: Caught deadly signal 'SIGTERM']]) })
+ screen:expect({ any = vim.pesc('[Process exited 1]') })
end)
it('no stack-use-after-scope with cursor color #22432', function()
