diff --git a/runtime/doc/gui.txt b/runtime/doc/gui.txt index 60e2b195c1..b39278d4e4 100644 --- a/runtime/doc/gui.txt +++ b/runtime/doc/gui.txt @@ -78,13 +78,13 @@ Restart Nvim This fails when changes have been made and Vim refuses to |abandon| the current buffer. - Note: This only works if the UI and server are on the same - system. + 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 embedded server irrespective of unsaved - buffers. + Force restarts the Nvim server, abandoning unsaved buffers. ------------------------------------------------------------------------------ GUI commands diff --git a/runtime/doc/ui.txt b/runtime/doc/ui.txt index e82b774366..462cce3f5b 100644 --- a/runtime/doc/ui.txt +++ b/runtime/doc/ui.txt @@ -248,10 +248,13 @@ the editor. Indicates to the UI that it must stop rendering the cursor. This event is misnamed and does not actually have anything to do with busyness. -["restart"] ~ - |:restart| command was used. The UI should stop-and-restart the Nvim - server using the same startup arguments |v:argv|, and reattach to the - new server. +["restart", progpath, argv] ~ + |:restart| command has been used and the Nvim server is about to exit. + The UI should wait for the server to exit, and then start a new server + using `progpath` as the full path to the Nvim executable |v:progpath| and + `argv` as its arguments |v:argv|, and reattach to the new server. + Note: |--embed| and |--headless| are excluded from `argv`, and the client + should decide itself whether to add either flag. ["suspend"] ~ |:suspend| command or |CTRL-Z| mapping is used. A terminal client (or diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c index aa9dc1098e..27c8acb40c 100644 --- a/src/nvim/api/ui.c +++ b/src/nvim/api/ui.c @@ -7,6 +7,7 @@ #include #include "klib/kvec.h" +#include "nvim/api/private/converter.h" #include "nvim/api/private/defs.h" #include "nvim/api/private/helpers.h" #include "nvim/api/private/validate.h" @@ -17,6 +18,7 @@ #include "nvim/channel.h" #include "nvim/channel_defs.h" #include "nvim/eval.h" +#include "nvim/eval/typval.h" #include "nvim/event/defs.h" #include "nvim/event/loop.h" #include "nvim/event/multiqueue.h" @@ -239,6 +241,47 @@ void nvim_ui_detach(uint64_t channel_id, Error *err) remote_ui_disconnect(channel_id); } +/// Sends a "restart" UI event to the UI on the given channel. +/// +/// @return false if there is no UI on the channel, otherwise true +bool remote_ui_restart(uint64_t channel_id, Error *err) +{ + RemoteUI *ui = pmap_get(uint64_t)(&connected_uis, channel_id); + if (!ui) { + api_set_error(err, kErrorTypeException, + "UI not attached to channel: %" PRId64, channel_id); + return false; + } + + MAXSIZE_TEMP_ARRAY(args, 2); + + ADD_C(args, CSTR_AS_OBJ(get_vim_var_str(VV_PROGPATH))); + + Arena arena = ARENA_EMPTY; + const list_T *l = get_vim_var_list(VV_ARGV); + int argc = tv_list_len(l); + assert(argc > 0); + Array argv = arena_array(&arena, (size_t)argc + 1); + bool had_minmin = false; + TV_LIST_ITER_CONST(l, li, { + const char *arg = tv_get_string(TV_LIST_ITEM_TV(li)); + if (argv.size > 0 && !had_minmin && strequal(arg, "--")) { + had_minmin = true; + } + // Exclude --embed/--headless from `argv`, as the client may start the server in a + // different way than how the server was originally started. + if (argv.size == 0 || had_minmin + || (!strequal(arg, "--embed") && !strequal(arg, "--headless"))) { + ADD_C(argv, CSTR_AS_OBJ(arg)); + } + }); + ADD_C(args, ARRAY_OBJ(argv)); + + push_call(ui, "restart", args); + arena_mem_free(arena_finish(&arena)); + return true; +} + // TODO(bfredl): use me to detach a specific ui from the server void remote_ui_stop(RemoteUI *ui) { diff --git a/src/nvim/api/ui_events.in.h b/src/nvim/api/ui_events.in.h index fe18e3875b..d060850958 100644 --- a/src/nvim/api/ui_events.in.h +++ b/src/nvim/api/ui_events.in.h @@ -29,8 +29,8 @@ void visual_bell(void) FUNC_API_SINCE(3); void flush(void) FUNC_API_SINCE(3) FUNC_API_REMOTE_IMPL; -void restart(void) - FUNC_API_SINCE(14) FUNC_API_REMOTE_ONLY FUNC_API_CLIENT_IMPL; +void restart(String progpath, Array argv) + FUNC_API_SINCE(14) FUNC_API_REMOTE_ONLY FUNC_API_REMOTE_IMPL FUNC_API_CLIENT_IMPL; void suspend(void) FUNC_API_SINCE(3); void set_title(String title) diff --git a/src/nvim/ex_cmds.lua b/src/nvim/ex_cmds.lua index bfccc2b8ad..39812d0311 100644 --- a/src/nvim/ex_cmds.lua +++ b/src/nvim/ex_cmds.lua @@ -2184,13 +2184,13 @@ M.cmds = { command = 'quitall', flags = bit.bor(BANG, TRLBAR), addr_type = 'ADDR_NONE', - func = 'ex_quit_all', + func = 'ex_quitall_or_restart', }, { command = 'qall', flags = bit.bor(BANG, TRLBAR, CMDWIN, LOCK_OK), addr_type = 'ADDR_NONE', - func = 'ex_quit_all', + func = 'ex_quitall_or_restart', }, { command = 'read', @@ -2250,7 +2250,7 @@ M.cmds = { command = 'restart', flags = bit.bor(BANG, TRLBAR), addr_type = 'ADDR_NONE', - func = 'ex_restart', + func = 'ex_quitall_or_restart', }, { command = 'retab', diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index b9ba8a1128..eb61195ef1 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -4828,16 +4828,23 @@ int before_quit_all(exarg_T *eap) } /// ":qall": try to quit all windows -static void ex_quit_all(exarg_T *eap) +/// ":restart": restart the Nvim server +static void ex_quitall_or_restart(exarg_T *eap) { if (before_quit_all(eap) == FAIL) { return; } exiting = true; - if (eap->forceit || !check_changed_any(false, false)) { + Error err = ERROR_INIT; + if ((eap->forceit || !check_changed_any(false, false)) + && (eap->cmdidx != CMD_restart || remote_ui_restart(current_ui, &err))) { getout(0); } not_exiting(); + if (ERROR_SET(&err)) { + emsg(err.msg); // UI disappeared already? + api_clear_error(&err); + } } /// ":close": close current window, unless it is the last one @@ -5592,31 +5599,6 @@ static void ex_detach(exarg_T *eap) } } -/// ":restart" command -/// Restarts the server by delegating the work to the UI. -static void ex_restart(exarg_T *eap) -{ - bool forceit = eap && eap->forceit; - - win_T *wp = curwin; - - // If any buffer is changed and not saved, we cannot restart. - // But if called using bang (!), we will force restart. - if ((!buf_hide(wp->w_buffer) - && check_changed(wp->w_buffer, (p_awa ? CCGD_AW : 0) - | (forceit ? CCGD_FORCEIT : 0) - | CCGD_EXCMD)) - || check_more(true, forceit) == FAIL - || check_changed_any(forceit, true)) { - if (!forceit) { - return; - } - } - - // Send an ui restart event. - ui_call_restart(); -} - /// ":mode": /// If no argument given, get the screen size and redraw. static void ex_mode(exarg_T *eap) diff --git a/src/nvim/main.c b/src/nvim/main.c index af275bde1e..a95406ce4b 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -334,7 +334,8 @@ int main(int argc, char **argv) if (use_builtin_ui && !remote_ui) { ui_client_forward_stdin = !stdin_isatty; - uint64_t rv = ui_client_start_server(params.argc, params.argv); + uint64_t rv = ui_client_start_server(get_vim_var_str(VV_PROGPATH), + (size_t)params.argc, params.argv); if (!rv) { fprintf(stderr, "Failed to start Nvim server!\n"); os_exit(1); diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c index 4dce4bb78a..1f8513baef 100644 --- a/src/nvim/msgpack_rpc/channel.c +++ b/src/nvim/msgpack_rpc/channel.c @@ -499,8 +499,13 @@ static void rpc_close_event(void **argv) if (!is_ui_client) { // Avoid hanging when there are no other UIs and a prompt is triggered on exit. remote_ui_disconnect(channel->id); + } else { + ui_client_may_restart_server(); + if (ui_client_channel_id != channel->id) { + // A new server has been started. Don't exit. + return; + } } - if (!channel->detach) { exit_on_closed_chan(channel->exit_status == -1 ? 0 : channel->exit_status); } diff --git a/src/nvim/ui_client.c b/src/nvim/ui_client.c index 121a2f1077..577e425211 100644 --- a/src/nvim/ui_client.c +++ b/src/nvim/ui_client.c @@ -7,12 +7,9 @@ #include "nvim/api/keysets_defs.h" #include "nvim/api/private/defs.h" -#include "nvim/api/private/dispatch.h" #include "nvim/api/private/helpers.h" #include "nvim/channel.h" #include "nvim/channel_defs.h" -#include "nvim/eval.h" -#include "nvim/eval/typval.h" #include "nvim/eval/typval_defs.h" #include "nvim/event/multiqueue.h" #include "nvim/globals.h" @@ -25,7 +22,6 @@ #include "nvim/msgpack_rpc/channel.h" #include "nvim/msgpack_rpc/channel_defs.h" #include "nvim/os/os.h" -#include "nvim/os/os_defs.h" #include "nvim/profile.h" #include "nvim/tui/tui.h" #include "nvim/tui/tui_defs.h" @@ -51,14 +47,13 @@ static bool ui_client_is_remote = false; #endif // uncrustify:on -uint64_t ui_client_start_server(int argc, char **argv) +uint64_t ui_client_start_server(const char *exepath, size_t argc, char **argv) { - varnumber_T exit_status; - char **args = xmalloc(((size_t)(2 + argc)) * sizeof(char *)); + char **args = xmalloc((2 + argc) * sizeof(char *)); int args_idx = 0; args[args_idx++] = xstrdup(argv[0]); args[args_idx++] = xstrdup("--embed"); - for (int i = 1; i < argc; i++) { + for (size_t i = 1; i < argc; i++) { args[args_idx++] = xstrdup(argv[i]); } args[args_idx++] = NULL; @@ -72,7 +67,8 @@ uint64_t ui_client_start_server(int argc, char **argv) #else bool detach = true; #endif - Channel *channel = channel_job_start(args, get_vim_var_str(VV_PROGPATH), + varnumber_T exit_status; + Channel *channel = channel_job_start(args, exepath, CALLBACK_READER_INIT, on_err, CALLBACK_NONE, false, true, true, detach, kChannelStdinPipe, NULL, 0, 0, NULL, &exit_status); @@ -287,57 +283,69 @@ void ui_client_event_raw_line(GridLineEvent *g) (const schar_T *)grid_line_buf_char, grid_line_buf_attr); } -/// Restarts the embedded server without killing the UI. +/// When a "restart" UI event is received, its arguments are saved here when +/// waiting for the server to exit. +static Array restart_args = ARRAY_DICT_INIT; +static bool restart_pending = false; + void ui_client_event_restart(Array args) { - // 1. Client-side server detach. - ui_client_detach(); + // NB: don't send nvim_ui_detach to server, as it may have already exited. + // ui_client_detach(); - // 2. Close ui client channel (auto kills the `nvim --embed` server due to self-exit). - const char *error; - bool success = channel_close(ui_client_channel_id, kChannelPartAll, &error); - if (!success) { - ELOG("%s", error); + // Save the arguments for ui_client_may_restart_server() later. + api_free_array(restart_args); + restart_args = copy_array(args, NULL); + restart_pending = true; +} + +/// Called when the current server has exited. +void ui_client_may_restart_server(void) +{ + if (!restart_pending) { return; } + restart_pending = false; - // 3. Get v:argv. - typval_T *tv = get_vim_var_tv(VV_ARGV); - if (tv->v_type != VAR_LIST || tv->vval.v_list == NULL) { - ELOG("failed to get vim var typval"); - return; + size_t argc; + char **argv = NULL; + if (restart_args.size < 2 + || restart_args.items[0].type != kObjectTypeString + || restart_args.items[1].type != kObjectTypeArray + || (argc = restart_args.items[1].data.array.size) < 1) { + ELOG("Error handling ui event 'restart'"); + goto cleanup; } - list_T *l = tv->vval.v_list; - int argc = tv_list_len(l); - // Assert to be positive for safe conversion to size_t. - assert(argc > 0); - - char **argv = xmalloc(sizeof(char *) * ((size_t)argc + 1)); - listitem_T *li = tv_list_first(l); - for (int i = 0; i < argc && li != NULL; i++, li = TV_LIST_ITEM_NEXT(l, li)) { - if (TV_LIST_ITEM_TV(li)->v_type == VAR_STRING && TV_LIST_ITEM_TV(li)->vval.v_string != NULL) { - argv[i] = TV_LIST_ITEM_TV(li)->vval.v_string; - } else { + // 1. Get executable path and command-line arguments. + const char *exepath = restart_args.items[0].data.string.data; + argv = xcalloc(argc + 1, sizeof(char *)); + for (size_t i = 0; i < argc; i++) { + if (restart_args.items[1].data.array.items[i].type == kObjectTypeString) { + argv[i] = restart_args.items[1].data.array.items[i].data.string.data; + } + if (argv[i] == NULL) { argv[i] = ""; } } - argv[argc] = NULL; - // 4. Start a new `nvim --embed` server. - uint64_t rv = ui_client_start_server(argc, argv); + // 2. Start a new `nvim --embed` server. + uint64_t rv = ui_client_start_server(exepath, argc, argv); if (!rv) { ELOG("failed to start nvim server"); goto cleanup; } - // 5. Client-side server re-attach. + // 3. Client-side server re-attach. ui_client_channel_id = rv; + ui_client_is_remote = false; ui_client_attach(tui_width, tui_height, tui_term, tui_rgb); ILOG("restarted server id=%" PRId64, rv); cleanup: xfree(argv); + api_free_array(restart_args); + restart_args = (Array)ARRAY_DICT_INIT; } #ifdef EXITFREE diff --git a/src/nvim/ui_client.h b/src/nvim/ui_client.h index 928bd4c0a5..9032135ddf 100644 --- a/src/nvim/ui_client.h +++ b/src/nvim/ui_client.h @@ -20,9 +20,6 @@ EXTERN uint64_t ui_client_channel_id INIT( = 0); // exit status from embedded nvim process EXTERN int ui_client_exit_status INIT( = 0); -// 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. - /// 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 1cafbaeafe..78c5e1fa34 100644 --- a/test/functional/terminal/tui_spec.lua +++ b/test/functional/terminal/tui_spec.lua @@ -168,11 +168,14 @@ describe('TUI :restart', function() n.check_close() end) + local server_pipe = new_pipename() local screen = tt.setup_child_nvim({ '-u', 'NONE', '-i', 'NONE', + '--listen', + server_pipe, '--cmd', 'colorscheme vim', '--cmd', @@ -189,6 +192,16 @@ describe('TUI :restart', function() {3:-- TERMINAL --} | ]] screen:expect(s0) + local server_session = n.connect(server_pipe) + local _, server_pid = server_session:request('nvim_call_function', 'getpid', {}) + + local function restart_pid_check() + server_session:close() + server_session = n.connect(server_pipe) + local _, new_pid = server_session:request('nvim_call_function', 'getpid', {}) + t.neq(server_pid, new_pid) + server_pid = new_pid + end tt.feed_data(':1restart\013') screen:expect({ any = vim.pesc('{8:E481: No range allowed}') }) @@ -199,6 +212,7 @@ describe('TUI :restart', function() -- Check ":restart" on an unmodified buffer. tt.feed_data(':restart\013') screen:expect(s0) + restart_pid_check() tt.feed_data('ithis will be removed\027') screen:expect([[ @@ -224,6 +238,21 @@ describe('TUI :restart', function() -- Check ":restart!". tt.feed_data(':restart!\013') screen:expect(s0) + restart_pid_check() + + tt.feed_data(':echo\n') + screen:expect([[ + ^ | + {4:~ }|*3 + {5:[No Name] }| + | + {3:-- TERMINAL --} | + ]]) + + -- 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) + restart_pid_check() screen:try_resize(60, 6) screen:expect([[ @@ -234,7 +263,7 @@ describe('TUI :restart', function() {3:-- TERMINAL --} | ]]) - --- Check that ":restart" uses the updated size after terminal resize + --- Check that ":restart" uses the updated size after terminal resize. tt.feed_data(':restart\013') screen:expect([[ ^ | @@ -243,6 +272,7 @@ describe('TUI :restart', function() {MATCH:%d+ +}| {3:-- TERMINAL --} | ]]) + restart_pid_check() end) end) @@ -3641,25 +3671,22 @@ describe('TUI client', function() }) feed_data('iHello, World') - screen_server:expect { - grid = [[ + screen_server:expect([[ Hello, World^ | {4:~ }|*3 {5:[No Name] [+] }| {3:-- INSERT --} | {3:-- TERMINAL --} | - ]], - } + ]]) feed_data('\027') - screen_server:expect { - grid = [[ + local s0 = [[ Hello, Worl^d | {4:~ }|*3 {5:[No Name] [+] }| | {3:-- TERMINAL --} | - ]], - } + ]] + screen_server:expect(s0) set_session(client_super) local screen_client = tt.setup_child_nvim({ @@ -3667,28 +3694,31 @@ describe('TUI client', function() '--server', server_pipe, }) - - screen_client:expect { - grid = [[ - Hello, Worl^d | - {4:~ }|*3 - {5:[No Name] [+] }| - | - {3:-- TERMINAL --} | - ]], - } + screen_client:expect(s0) -- grid smaller than containing terminal window is cleared properly feed_data(":call setline(1,['a'->repeat(&columns)]->repeat(&lines))\n") feed_data('0:set lines=3\n') - screen_server:expect { - grid = [[ + local s1 = [[ ^aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa| {5:[No Name] [+] }| |*4 {3:-- TERMINAL --} | - ]], - } + ]] + screen_client:expect(s1) + screen_server:expect(s1) + + -- Run :restart! on the remote client. + -- The remote client should start a new server while the original one should exit. + feed_data(':restart!\n') + screen_client:expect([[ + ^ | + {4:~ }|*3 + {5:[No Name] }| + | + {3:-- TERMINAL --} | + ]]) + screen_server:expect({ any = vim.pesc('[Process exited 0]') }) feed_data(':q!\n') @@ -3712,33 +3742,53 @@ describe('TUI client', function() server_pipe, }) - screen_client:expect { - grid = [[ + screen_client:expect([[ Halloj^! | {4:~ }|*4 | {3:-- TERMINAL --} | - ]], - } + ]]) -- No heap-use-after-free when receiving UI events after deadly signal #22184 server:request('nvim_input', ('a'):rep(1000)) exec_lua([[vim.uv.kill(vim.fn.jobpid(vim.bo.channel), 'sigterm')]]) - screen_client:expect { - grid = [[ + screen_client:expect([[ Nvim: Caught deadly signal 'SIGTERM' | | [Process exited 1]^ | |*3 {3:-- TERMINAL --} | - ]], - } + ]]) eq(0, api.nvim_get_vvar('shell_error')) -- exits on input eof #22244 fn.system({ nvim_prog, '--remote-ui', '--server', server_pipe }) eq(1, api.nvim_get_vvar('shell_error')) + command('bwipe!') + fn.jobstart({ nvim_prog, '--remote-ui', '--server', server_pipe }, { term = true }) + command('startinsert') + screen_client:expect([[ + {4:<<<}aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa| + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|*3 + aaaaaa^ | + {3:-- INSERT --} | + {3:-- TERMINAL --} | + ]]) + + -- Run :restart! on the client. + -- The client should start a new server while the original server should exit. + feed_data('\027:restart!\n') + screen_client:expect([[ + ^ | + {4:~ }|*4 + | + {3:-- TERMINAL --} | + ]]) + retry(nil, nil, function() + eq(nil, vim.uv.fs_stat(server_pipe)) + end) + client_super:close() server:close() if is_os('mac') then