diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c index aa9dc1098e..d2e0fd98a8 100644 --- a/src/nvim/api/ui.c +++ b/src/nvim/api/ui.c @@ -71,12 +71,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); @@ -231,12 +245,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); } // TODO(bfredl): use me to detach a specific ui from the server diff --git a/src/nvim/api/ui_events.in.h b/src/nvim/api/ui_events.in.h index 74718e7ac5..f5b9f784d3 100644 --- a/src/nvim/api/ui_events.in.h +++ b/src/nvim/api/ui_events.in.h @@ -173,5 +173,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 index e02f9eb6cd..b62ac336e4 100644 --- a/src/nvim/event/proc.c +++ b/src/nvim/event/proc.c @@ -5,6 +5,7 @@ #include #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,20 @@ 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) { + // If the current embedded server has exited, + // 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 index 66c571246a..4b729b938c 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -5553,8 +5553,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) { @@ -5571,7 +5571,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 index 4dce4bb78a..f5a4d69a9e 100644 --- a/src/nvim/msgpack_rpc/channel.c +++ b/src/nvim/msgpack_rpc/channel.c @@ -498,18 +498,22 @@ 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); } - 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 index 41994cc935..bb91b5336a 100644 --- a/src/nvim/tui/tui.c +++ b/src/nvim/tui/tui.c @@ -147,7 +147,6 @@ struct TUIData { char *space_buf; size_t space_buf_len; bool stopped; - int seen_error_exit; int width; int height; bool rgb; @@ -170,7 +169,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; @@ -558,14 +556,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); } @@ -604,12 +604,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 { @@ -650,8 +644,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 index b256c1777f..ad65d1566d 100644 --- a/src/nvim/ui_client.c +++ b/src/nvim/ui_client.c @@ -284,6 +284,16 @@ void ui_client_event_raw_line(GridLineEvent *g) (const schar_T *)grid_line_buf_char, grid_line_buf_attr); } +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 index 928bd4c0a5..7022e43786 100644 --- a/src/nvim/ui_client.h +++ b/src/nvim/ui_client.h @@ -17,11 +17,12 @@ 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 -EXTERN int ui_client_exit_status INIT( = 0); +/// `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); -// TODO(bfredl): the current structure for how tui and ui_client.c communicate is a bit awkward. -// This will be restructured as part of The UI Devirtualization Project. +/// Server exit code. +EXTERN int ui_client_exit_status INIT( = 0); /// Whether ui client has sent nvim_ui_attach yet EXTERN bool ui_client_attached INIT( = false); diff --git a/test/functional/terminal/tui_spec.lua b/test/functional/terminal/tui_spec.lua index 33b34dca97..2b5ce0f120 100644 --- 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() before_each(function() os.remove(testlog) @@ -2119,15 +2141,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()