diff --git a/runtime/doc/news.txt b/runtime/doc/news.txt index 4f66320f27..631eec56ce 100644 --- a/runtime/doc/news.txt +++ b/runtime/doc/news.txt @@ -426,6 +426,11 @@ These existing features changed their behavior. - Windows: Paths like "\Windows" and "/Windows" are now considered to be absolute paths (to the current drive) and no longer relative. +• When 'shelltemp' is off, shell commands now use `pipe()` and not `socketpair()` + for input and output. This matters mostly for Linux where some command lines + using "/dev/stdin" and similiar would break as these special files can be + reopened when backed by pipes but not when backed by socket pairs. + ============================================================================== REMOVED FEATURES *news-removed* diff --git a/src/nvim/event/libuv_proc.c b/src/nvim/event/libuv_proc.c index b090367f92..7405892af7 100644 --- a/src/nvim/event/libuv_proc.c +++ b/src/nvim/event/libuv_proc.c @@ -60,27 +60,58 @@ int libuv_proc_spawn(LibuvProc *uvproc) uvproc->uvopts.env = NULL; } + int to_close[3] = { -1, -1, -1 }; + if (!proc->in.closed) { - uvproc->uvstdio[0].flags = UV_CREATE_PIPE | UV_READABLE_PIPE; + uv_file pipe_pair[2]; + int client_flags = 0; #ifdef MSWIN - uvproc->uvstdio[0].flags |= proc->overlapped ? UV_OVERLAPPED_PIPE : 0; + client_flags |= proc->overlapped ? UV_NONBLOCK_PIPE : 0; #endif - uvproc->uvstdio[0].data.stream = (uv_stream_t *)(&proc->in.uv.pipe); + + // As of libuv 1.51, UV_CREATE_PIPE can only create pipes + // using socketpair(), not pipe(). We want the latter on linux + // as socket pairs behave different in some confusing ways, like + // breaking /proc/0/fd/0 which is disowned by the linux socket maintainer. + uv_pipe(pipe_pair, client_flags, UV_NONBLOCK_PIPE); + + uvproc->uvstdio[0].flags = UV_INHERIT_FD; + uvproc->uvstdio[0].data.fd = pipe_pair[0]; + to_close[0] = pipe_pair[0]; + + uv_pipe_open(&proc->in.uv.pipe, pipe_pair[1]); } if (!proc->out.s.closed) { - uvproc->uvstdio[1].flags = UV_CREATE_PIPE | UV_WRITABLE_PIPE; #ifdef MSWIN + // TODO(bfredl): in theory it would have been nice if the uv_pipe() branch + // also worked for windows but IOCP happens because of reasons. + uvproc->uvstdio[1].flags = UV_CREATE_PIPE | UV_WRITABLE_PIPE; // pipe must be readable for IOCP to work on Windows. uvproc->uvstdio[1].flags |= proc->overlapped ? (UV_READABLE_PIPE | UV_OVERLAPPED_PIPE) : 0; -#endif uvproc->uvstdio[1].data.stream = (uv_stream_t *)(&proc->out.s.uv.pipe); +#else + uv_file pipe_pair[2]; + uv_pipe(pipe_pair, UV_NONBLOCK_PIPE, 0); + + uvproc->uvstdio[1].flags = UV_INHERIT_FD; + uvproc->uvstdio[1].data.fd = pipe_pair[1]; + to_close[1] = pipe_pair[1]; + + uv_pipe_open(&proc->out.s.uv.pipe, pipe_pair[0]); +#endif } if (!proc->err.s.closed) { - uvproc->uvstdio[2].flags = UV_CREATE_PIPE | UV_WRITABLE_PIPE; - uvproc->uvstdio[2].data.stream = (uv_stream_t *)(&proc->err.s.uv.pipe); + uv_file pipe_pair[2]; + uv_pipe(pipe_pair, UV_NONBLOCK_PIPE, 0); + + uvproc->uvstdio[2].flags = UV_INHERIT_FD; + uvproc->uvstdio[2].data.fd = pipe_pair[1]; + to_close[2] = pipe_pair[1]; + + uv_pipe_open(&proc->err.s.uv.pipe, pipe_pair[0]); } else if (proc->fwd_err) { uvproc->uvstdio[2].flags = UV_INHERIT_FD; uvproc->uvstdio[2].data.fd = STDERR_FILENO; @@ -92,10 +123,16 @@ int libuv_proc_spawn(LibuvProc *uvproc) if (uvproc->uvopts.env) { os_free_fullenv(uvproc->uvopts.env); } - return status; + goto exit; } proc->pid = uvproc->uv.pid; +exit: + for (int i = 0; i < 3; i++) { + if (to_close[i] > -1) { + close(to_close[i]); + } + } return status; } diff --git a/test/client/uv_stream.lua b/test/client/uv_stream.lua index 73e3255f6f..29918c576a 100644 --- a/test/client/uv_stream.lua +++ b/test/client/uv_stream.lua @@ -150,9 +150,10 @@ ProcStream.__index = ProcStream --- @param env string[]? --- @param io_extra uv.uv_pipe_t? --- @param on_exit fun(closed: integer?)? Called after the child process exits. +--- @param forward_stderr forward stderr from the nvim process. otherwise collect it. --- `closed` is the timestamp (uv.now()) when close() was called, or nil if it wasn't. --- @return test.ProcStream -function ProcStream.spawn(argv, env, io_extra, on_exit) +function ProcStream.spawn(argv, env, io_extra, on_exit, forward_stderr) local self = setmetatable({ collect_text = false, output = function(self) @@ -176,9 +177,10 @@ function ProcStream.spawn(argv, env, io_extra, on_exit) for i = 2, #argv do args[#args + 1] = argv[i] end + local stderr = forward_stderr and 1 or self._child_stderr --- @diagnostic disable-next-line:missing-fields self._proc, self._pid = uv.spawn(prog, { - stdio = { self._child_stdin, self._child_stdout, self._child_stderr, io_extra }, + stdio = { self._child_stdin, self._child_stdout, stderr, io_extra }, args = args, --- @diagnostic disable-next-line:assign-type-mismatch env = env, diff --git a/test/functional/core/job_spec.lua b/test/functional/core/job_spec.lua index ee7f7e5a27..787262fd64 100644 --- a/test/functional/core/job_spec.lua +++ b/test/functional/core/job_spec.lua @@ -1339,6 +1339,30 @@ describe('jobs', function() ]]) end end) + + it('uses real pipes for stdin/stdout #35984', function() + if skip(is_os('win'), 'Not applicable for Windows') then + return + end + -- this fails on linux if we used socketpair() for stdin and stdout, + -- which libuv does if you ask to create stdio streams for you + local val = exec_lua(function() + local output + local job = vim.fn.jobstart('wc /dev/stdin > /dev/stdout', { + stdout_buffered = true, + on_stdout = function(_, data, _) + output = data + end, + }) + vim.fn.chansend(job, 'foo\nbar baz\n') + vim.fn.chanclose(job, 'stdin') + vim.fn.jobwait({ job }) + return output + end) + eq(2, #val, val) + eq({ '2', '3', '12', '/dev/stdin' }, vim.split(val[1], '%s+', { trimempty = true })) + eq('', val[2]) + end) end) describe('pty process teardown', function() diff --git a/test/functional/testnvim.lua b/test/functional/testnvim.lua index 7d03659f5d..75c48f36a6 100644 --- a/test/functional/testnvim.lua +++ b/test/functional/testnvim.lua @@ -515,7 +515,7 @@ function M.new_session(keep, ...) ) io.stdout:flush() end - end) + end, true) n_processes = n_processes + 1 local new_session = Session.new(proc) diff --git a/test/functional/vimscript/system_spec.lua b/test/functional/vimscript/system_spec.lua index e075181460..54650caa71 100644 --- a/test/functional/vimscript/system_spec.lua +++ b/test/functional/vimscript/system_spec.lua @@ -196,10 +196,10 @@ describe('system()', function() n.set_shell_powershell() eq('ああ\n', eval([[system('Write-Output "ああ"')]])) -- Sanity test w/ default encoding - -- * on Windows, expected to default to Western European enc + -- * on Windows, UTF-8 still works. -- * on Linux, expected to default to UTF8 command([[let &shellcmdflag = '-NoLogo -NoProfile -ExecutionPolicy RemoteSigned -Command ']]) - eq(is_os('win') and '??\n' or 'ああ\n', eval([[system('Write-Output "ああ"')]])) + eq('ああ\n', eval([[system('Write-Output "ああ"')]])) end) it('`echo` and waits for its return', function() @@ -548,10 +548,10 @@ describe('systemlist()', function() n.set_shell_powershell() eq({ is_os('win') and 'あ\r' or 'あ' }, eval([[systemlist('Write-Output あ')]])) -- Sanity test w/ default encoding - -- * on Windows, expected to default to Western European enc + -- * on Windows, UTF-8 still works. -- * on Linux, expected to default to UTF8 command([[let &shellcmdflag = '-NoLogo -NoProfile -ExecutionPolicy RemoteSigned -Command ']]) - eq({ is_os('win') and '?\r' or 'あ' }, eval([[systemlist('Write-Output あ')]])) + eq({ is_os('win') and 'あ\r' or 'あ' }, eval([[systemlist('Write-Output あ')]])) end) end)