From 3aada5982c368f075dd0014dd9bdd5b8d2947b51 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 1 Sep 2021 15:46:28 -0400 Subject: [PATCH] Reland "change setup_stdio default method to support any IO (#40780)" (#40971) (#40973) This reverts commit 7edd1904b9be0088c65dfd4e8520a976db6ee5a3, and fixes a method type signature error, which caused TTY to get shutdown. --- base/cmd.jl | 1 + base/filesystem.jl | 3 +- base/process.jl | 69 ++++++++++++++++++++++------------------------ base/stream.jl | 2 ++ test/spawn.jl | 6 ++++ 5 files changed, 44 insertions(+), 37 deletions(-) diff --git a/base/cmd.jl b/base/cmd.jl index ff52191fc51ff..0c2a22e6cf852 100644 --- a/base/cmd.jl +++ b/base/cmd.jl @@ -167,6 +167,7 @@ rawhandle(x::OS_HANDLE) = x if OS_HANDLE !== RawFD rawhandle(x::RawFD) = Libc._get_osfhandle(x) end +setup_stdio(stdio::Union{DevNull,OS_HANDLE,RawFD}, ::Bool) = (stdio, false) const Redirectable = Union{IO, FileRedirect, RawFD, OS_HANDLE} const StdIOSet = NTuple{3, Redirectable} diff --git a/base/filesystem.jl b/base/filesystem.jl index 569b71995688d..dfa881068c6ab 100644 --- a/base/filesystem.jl +++ b/base/filesystem.jl @@ -58,7 +58,7 @@ import .Base: IOError, _UVError, _sizeof_uv_fs, check_open, close, eof, eventloop, fd, isopen, bytesavailable, position, read, read!, readavailable, seek, seekend, show, skip, stat, unsafe_read, unsafe_write, write, transcode, uv_error, - rawhandle, OS_HANDLE, INVALID_OS_HANDLE, windowserror, filesize + setup_stdio, rawhandle, OS_HANDLE, INVALID_OS_HANDLE, windowserror, filesize import .Base.RefValue @@ -92,6 +92,7 @@ if OS_HANDLE !== RawFD end rawhandle(file::File) = file.handle +setup_stdio(file::File, ::Bool) = (file, false) # Filesystem.open, not Base.open function open(path::AbstractString, flags::Integer, mode::Integer=0) diff --git a/base/process.jl b/base/process.jl index 666c94c49a7fa..10c173e82b34c 100644 --- a/base/process.jl +++ b/base/process.jl @@ -74,27 +74,29 @@ const SpawnIOs = Vector{Any} # convenience name for readability # handle marshalling of `Cmd` arguments from Julia to C @noinline function _spawn_primitive(file, cmd::Cmd, stdio::SpawnIOs) loop = eventloop() - iohandles = Tuple{Cint, UInt}[ # assuming little-endian layout - let h = rawhandle(io) - h === C_NULL ? (0x00, UInt(0)) : - h isa OS_HANDLE ? (0x02, UInt(cconvert(@static(Sys.iswindows() ? Ptr{Cvoid} : Cint), h))) : - h isa Ptr{Cvoid} ? (0x04, UInt(h)) : - error("invalid spawn handle $h from $io") - end - for io in stdio] - handle = Libc.malloc(_sizeof_uv_process) - disassociate_julia_struct(handle) # ensure that data field is set to C_NULL - (; exec, flags, env, dir) = cmd - err = ccall(:jl_spawn, Int32, - (Cstring, Ptr{Cstring}, Ptr{Cvoid}, Ptr{Cvoid}, - Ptr{Tuple{Cint, UInt}}, Int, - UInt32, Ptr{Cstring}, Cstring, Ptr{Cvoid}), - file, exec, loop, handle, - iohandles, length(iohandles), - flags, - env === nothing ? C_NULL : env, - isempty(dir) ? C_NULL : dir, - @cfunction(uv_return_spawn, Cvoid, (Ptr{Cvoid}, Int64, Int32))) + GC.@preserve stdio begin + iohandles = Tuple{Cint, UInt}[ # assuming little-endian layout + let h = rawhandle(io) + h === C_NULL ? (0x00, UInt(0)) : + h isa OS_HANDLE ? (0x02, UInt(cconvert(@static(Sys.iswindows() ? Ptr{Cvoid} : Cint), h))) : + h isa Ptr{Cvoid} ? (0x04, UInt(h)) : + error("invalid spawn handle $h from $io") + end + for io in stdio] + handle = Libc.malloc(_sizeof_uv_process) + disassociate_julia_struct(handle) # ensure that data field is set to C_NULL + (; exec, flags, env, dir) = cmd + err = ccall(:jl_spawn, Int32, + (Cstring, Ptr{Cstring}, Ptr{Cvoid}, Ptr{Cvoid}, + Ptr{Tuple{Cint, UInt}}, Int, + UInt32, Ptr{Cstring}, Cstring, Ptr{Cvoid}), + file, exec, loop, handle, + iohandles, length(iohandles), + flags, + env === nothing ? C_NULL : env, + isempty(dir) ? C_NULL : dir, + @cfunction(uv_return_spawn, Cvoid, (Ptr{Cvoid}, Int64, Int32))) + end if err != 0 ccall(:jl_forceclose_uv, Cvoid, (Ptr{Cvoid},), handle) # will call free on handle eventually throw(_UVError("could not spawn " * repr(cmd), err)) @@ -210,10 +212,10 @@ function setup_stdio(stdio::PipeEndpoint, child_readable::Bool) rd, wr = link_pipe(!child_readable, child_readable) try open_pipe!(stdio, child_readable ? wr : rd) - catch ex + catch close_pipe_sync(rd) close_pipe_sync(wr) - rethrow(ex) + rethrow() end child = child_readable ? rd : wr return (child, true) @@ -252,18 +254,19 @@ function setup_stdio(stdio::FileRedirect, child_readable::Bool) return (io, true) end -# incrementally move data between an IOBuffer and a system Pipe +# incrementally move data between an arbitrary IO and a system Pipe, +# including copying the EOF (shutdown) when finished # TODO: probably more efficient (when valid) to use `stdio` directly as the # PipeEndpoint buffer field in some cases -function setup_stdio(stdio::Union{IOBuffer, BufferStream}, child_readable::Bool) +function setup_stdio(stdio::IO, child_readable::Bool) parent = PipeEndpoint() rd, wr = link_pipe(!child_readable, child_readable) try open_pipe!(parent, child_readable ? wr : rd) - catch ex + catch close_pipe_sync(rd) close_pipe_sync(wr) - rethrow(ex) + rethrow() end child = child_readable ? rd : wr try @@ -272,25 +275,19 @@ function setup_stdio(stdio::Union{IOBuffer, BufferStream}, child_readable::Bool) @async try write(in, out) catch ex - @warn "Process error" exception=(ex, catch_backtrace()) + @warn "Process I/O error" exception=(ex, catch_backtrace()) finally close(parent) child_readable || closewrite(stdio) end end - catch ex + catch close_pipe_sync(child) - rethrow(ex) + rethrow() end return (child, true) end -function setup_stdio(io, child_readable::Bool) - # if there is no specialization, - # assume that rawhandle is defined for it - return (io, false) -end - close_stdio(stdio::OS_HANDLE) = close_pipe_sync(stdio) close_stdio(stdio) = close(stdio) diff --git a/base/stream.jl b/base/stream.jl index b9bfa7aee1d96..05178a7f9ebcb 100644 --- a/base/stream.jl +++ b/base/stream.jl @@ -283,6 +283,7 @@ end lock(s::LibuvStream) = lock(s.lock) unlock(s::LibuvStream) = unlock(s.lock) +setup_stdio(stream::LibuvStream, ::Bool) = (stream, false) rawhandle(stream::LibuvStream) = stream.handle unsafe_convert(::Type{Ptr{Cvoid}}, s::Union{LibuvStream, LibuvServer}) = s.handle @@ -1488,6 +1489,7 @@ function close(s::BufferStream) end end uvfinalize(s::BufferStream) = nothing +setup_stdio(stream::BufferStream, child_readable::Bool) = invoke(setup_stdio, Tuple{IO, Bool}, stream, child_readable) function read(s::BufferStream, ::Type{UInt8}) nread = lock(s.cond) do diff --git a/test/spawn.jl b/test/spawn.jl index a6b919e7a9ad1..ab8accf65e64a 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -771,6 +771,12 @@ let text = "input-test-text" @test proc.out === out @test read(out, String) == text @test success(proc) + + out = PipeBuffer() + proc = run(catcmd, IOBuffer(SubString(text)), out) + @test success(proc) + @test proc.out === proc.err === proc.in === devnull + @test String(take!(out)) == text end