From b40ca5a01c39c2acf10ff8bcca9db18b3336540c Mon Sep 17 00:00:00 2001 From: Sanzhar Kuandyk <92693103+SanzharKuandyk@users.noreply.github.com> Date: Sat, 28 Feb 2026 18:21:13 +0500 Subject: [PATCH] fix(channel): support :detach, :restart on Windows #37977 fix: allocate hidden console for detached server Starting the server with UV_PROCESS_DETACHED results in DETACHED_PROCESS, leaving the child without a console. Without a console: CONIN$ / CONOUT$ cannot resolve, causing channel_from_stdio to fail. ConPTY cannot attach, breaking :terminal. This patch allocates a hidden console via AllocConsole() when the server has none, restoring working stdio and enabling ConPTY. Also updates os_set_cloexec to clear HANDLE_FLAG_INHERIT on the RPC pipe handles, matching the Unix F_DUPFD_CLOEXEC behavior. --- src/nvim/channel.c | 31 ++++++++++++- src/nvim/main.c | 36 ++++++++++++++ src/nvim/os/fs.c | 20 +++++--- src/nvim/ui_client.c | 5 -- test/functional/terminal/tui_spec.lua | 67 ++++++--------------------- test/functional/testterm.lua | 14 ++++++ 6 files changed, 107 insertions(+), 66 deletions(-) diff --git a/src/nvim/channel.c b/src/nvim/channel.c index 465c678e5d..ea564a052a 100644 --- a/src/nvim/channel.c +++ b/src/nvim/channel.c @@ -407,6 +407,14 @@ Channel *channel_job_start(char **argv, const char *exepath, CallbackReader on_s has_out = rpc || callback_reader_set(chan->on_data); has_err = callback_reader_set(chan->on_stderr); proc->fwd_err = chan->on_stderr.fwd_err; +#ifdef MSWIN + // DETACHED_PROCESS can't inherit console handles (like ConPTY stderr). + // Use a pipe relay: libuv creates a pipe, on_channel_output writes to stderr. + if (!has_err && proc->fwd_err && proc->detach) { + has_err = true; + proc->fwd_err = false; + } +#endif } bool has_in = stdin_mode == kChannelStdinPipe; @@ -536,8 +544,18 @@ uint64_t channel_from_stdio(bool rpc, CallbackReader on_output, const char **err // stdin and stdout with CONIN$ and CONOUT$, respectively. if (embedded_mode && os_has_conpty_working()) { stdin_dup_fd = os_dup(STDIN_FILENO); - os_replace_stdin_to_conin(); + os_set_cloexec(stdin_dup_fd); stdout_dup_fd = os_dup(STDOUT_FILENO); + os_set_cloexec(stdout_dup_fd); + + // The server may have no console (spawned with UV_PROCESS_DETACHED for + // :detach support). Allocate a hidden one so CONIN$/CONOUT$ and ConPTY + // (:terminal) work. + if (!GetConsoleWindow()) { + AllocConsole(); + ShowWindow(GetConsoleWindow(), SW_HIDE); + } + os_replace_stdin_to_conin(); os_replace_stdout_and_stderr_to_conout(); } #else @@ -668,6 +686,17 @@ static size_t on_channel_output(RStream *stream, Channel *chan, const char *buf, reader->eof = true; } +#ifdef MSWIN + // Pipe relay for fwd_err on Windows: relay server stderr to stdout. + // DETACHED_PROCESS prevents inheriting console handles, so channel_job_start + // creates a pipe instead. Write to stdout because ConPTY only captures stdout. + // On non-Windows, fwd_err uses UV_INHERIT_FD directly; this path is never reached. + if (reader->fwd_err && count > 0) { + os_write(STDOUT_FILENO, buf, count, false); + return count; + } +#endif + if (callback_reader_set(*reader)) { ga_concat_len(&reader->buffer, buf, count); schedule_channel_event(chan); diff --git a/src/nvim/main.c b/src/nvim/main.c index 9c3ae2e5b5..6d4e036a20 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -306,6 +306,19 @@ int main(int argc, char **argv) nlua_init(argv, argc, params.lua_arg0); TIME_MSG("init lua interpreter"); + // On Windows, channel_from_stdio() replaces fd 2 with CONOUT$ (for ConPTY + // support). Save a dup of the original stderr first so that if server_init() + // fails, print_mainerr() can write through the pipe to the TUI client's relay. +#ifdef MSWIN + int startup_stderr_fd = -1; + if (embedded_mode) { + startup_stderr_fd = os_dup(STDERR_FILENO); + if (startup_stderr_fd >= 0) { + os_set_cloexec(startup_stderr_fd); + } + } +#endif + if (embedded_mode) { const char *err; if (!channel_from_stdio(true, CALLBACK_READER_INIT, &err)) { @@ -357,9 +370,27 @@ int main(int argc, char **argv) // Nvim server... if (!server_init(params.listen_addr)) { +#ifdef MSWIN + // Restore the original stderr (pipe to TUI client) so print_mainerr() + // output is visible in the TUI terminal via the relay in on_channel_output. + if (startup_stderr_fd >= 0) { + dup2(startup_stderr_fd, STDERR_FILENO); + close(startup_stderr_fd); + startup_stderr_fd = -1; + } +#endif mainerr(IObuff, NULL, NULL); } +#ifdef MSWIN + // Server started successfully. Close the saved fd so the pipe write end is + // fully released — child processes inherit CONOUT$ (fd 2), not the pipe. + if (startup_stderr_fd >= 0) { + close(startup_stderr_fd); + startup_stderr_fd = -1; + } +#endif + TIME_MSG("expanding arguments"); if (params.diff_mode && params.window_count == -1) { @@ -957,6 +988,11 @@ static void remote_request(mparm_T *params, int remote_args, char *server_addr, if (is_ui) { if (!chan) { +#ifdef MSWIN + // The TUI client is spawned in a ConPTY which only captures stdout. + // Redirect stderr to stdout so this error appears in the terminal. + dup2(STDOUT_FILENO, STDERR_FILENO); +#endif fprintf(stderr, "Remote ui failed to start: %s\n", connect_error); os_exit(1); } else if (strequal(server_addr, os_getenv_noalloc("NVIM"))) { diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c index 93a3750a7b..8ad75ccc1a 100644 --- a/src/nvim/os/fs.c +++ b/src/nvim/os/fs.c @@ -13,6 +13,7 @@ #include #ifdef MSWIN +# include # include #endif @@ -483,9 +484,9 @@ FILE *os_fopen(const char *path, const char *flags) return fdopen(fd, flags); } -/// Sets file descriptor `fd` to close-on-exec. -// -// @return -1 if failed to set, 0 otherwise. +/// Sets file descriptor `fd` to close-on-exec (Unix) or non-inheritable (Windows). +/// +/// @return -1 if failed to set, 0 otherwise. int os_set_cloexec(const int fd) { #ifdef HAVE_FD_CLOEXEC @@ -505,11 +506,16 @@ int os_set_cloexec(const int fd) return -1; } return 0; -#endif - - // No FD_CLOEXEC flag. On Windows, the file should have been opened with - // O_NOINHERIT anyway. +#elif defined(MSWIN) + HANDLE h = (HANDLE)_get_osfhandle(fd); + if (h == INVALID_HANDLE_VALUE + || !SetHandleInformation(h, HANDLE_FLAG_INHERIT, 0)) { + return -1; + } + return 0; +#else return -1; +#endif } /// Close a file diff --git a/src/nvim/ui_client.c b/src/nvim/ui_client.c index 07d22d49c1..ab4eecec9a 100644 --- a/src/nvim/ui_client.c +++ b/src/nvim/ui_client.c @@ -60,12 +60,7 @@ uint64_t ui_client_start_server(const char *exepath, size_t argc, char **argv) CallbackReader on_err = CALLBACK_READER_INIT; on_err.fwd_err = true; -#ifdef MSWIN - // TODO(justinmk): detach breaks `tt.setup_child_nvim` tests on Windows? - bool detach = os_env_exists("__NVIM_DETACH", true); -#else bool detach = true; -#endif varnumber_T exit_status; Channel *channel = channel_job_start(args, exepath, CALLBACK_READER_INIT, on_err, CALLBACK_NONE, diff --git a/test/functional/terminal/tui_spec.lua b/test/functional/terminal/tui_spec.lua index 80e8f33b01..981e0c0c47 100644 --- a/test/functional/terminal/tui_spec.lua +++ b/test/functional/terminal/tui_spec.lua @@ -50,18 +50,20 @@ describe('TUI', function() end) screen:expect({ any = vim.pesc('[Process exited 1]') }) + -- 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' + fn.fnamemodify(nvim_prog, ':t') ) ), unchanged = true, }) end + -- Always assert the log for the error message. assert_log( vim.pesc('Failed to start server: address already in use: ' .. addr_in_use), testlog, @@ -116,41 +118,6 @@ describe('TUI :detach', function() it('does not stop server', function() local job_opts = { env = t.shallowcopy(env_notermguicolors) } - if is_os('win') then - -- TODO(justinmk): on Windows, - -- - tt.setup_child_nvim() is broken. - -- - session.lua is broken after the pipe closes. - -- So this test currently just exercises __NVIM_DETACH + :detach, without asserting anything. - - -- TODO(justinmk): temporary hack for Windows. - job_opts.env['__NVIM_DETACH'] = '1' - n.clear(job_opts) - - local screen = Screen.new(50, 10) - n.feed('iHello, World') - screen:expect([[ - Hello, World^ | - {1:~ }|*8 - {5:-- INSERT --} | - ]]) - - -- local addr = api.nvim_get_vvar('servername') - eq(1, #n.api.nvim_list_uis()) - - -- TODO(justinmk): test util should not freak out when the pipe closes. - n.expect_exit(n.command, 'detach') - - -- n.get_session():close() -- XXX: hangs - -- n.set_session(n.connect(addr)) -- XXX: hangs - -- eq(0, #n.api.nvim_list_uis()) -- XXX: hangs - - -- Avoid a dangling process. - n.get_session():close('kill') - -- n.expect_exit(n.command, 'qall!') - - return - end - n.clear() finally(function() n.check_close() @@ -171,13 +138,16 @@ describe('TUI :detach', function() }, job_opts) tt.feed_data('iHello, World') - screen:expect([[ + tt.screen_expect( + screen, + [[ Hello, World^ | {100:~ }|*3 {3:[No Name] [+] }| {5:-- INSERT --} | {5:-- TERMINAL --} | - ]]) + ]] + ) local child_session = n.connect(child_server) finally(function() @@ -226,13 +196,16 @@ describe('TUI :detach', function() child_server, }, job_opts) - screen_reattached:expect([[ + tt.screen_expect( + screen_reattached, + [[ We did it, pooky^. | {100:~ }|*3 {3:[No Name] [+] }| | {5:-- TERMINAL --} | - ]]) + ]] + ) end) end) @@ -264,16 +237,8 @@ describe('TUI :restart', function() 'echo getpid()', }, { env = env_notermguicolors }) - --- 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') - end - screen:expect(s) + tt.screen_expect(screen, s) end -- The value of has("gui_running") should be 0 before and after :restart. @@ -499,10 +464,6 @@ describe('TUI :restart', function() end) describe('TUI :connect', function() - if t.skip(is_os('win'), "relies on :detach which currently doesn't work on windows") then - return - end - local screen_empty = [[ ^ | {100:~ }|*5 diff --git a/test/functional/testterm.lua b/test/functional/testterm.lua index 161dff8544..92cab6e540 100644 --- a/test/functional/testterm.lua +++ b/test/functional/testterm.lua @@ -8,6 +8,7 @@ -- - NOTE: Only use this if your test actually needs the full lifecycle/capabilities of the -- builtin Nvim TUI. Most tests should just use `Screen.new()` directly, or plain old API calls. +local t = require('test.testutil') local n = require('test.functional.testnvim')() local Screen = require('test.functional.ui.screen') @@ -207,4 +208,17 @@ function M.setup_child_nvim(args, opts) return M.setup_screen(opts.extra_rows, argv, opts.cols, env) end +--- FIXME: On Windows spaces at the end of a screen line may have wrong attrs. +--- Remove this function when that's fixed. +--- +--- @param screen test.functional.ui.screen +--- @param s string +function M.screen_expect(screen, s) + if t.is_os('win') then + s = s:gsub(' *%} +%|\n', '{MATCH: *}}{MATCH: *}|\n') + s = s:gsub('%}%^ +%|\n', '{MATCH:[ ^]*}}{MATCH:[ ^]*}|\n') + end + screen:expect(s) +end + return M