Skip to content

Commit

Permalink
Reland "change setup_stdio default method to support any IO (#40780)" (
Browse files Browse the repository at this point in the history
…#40971) (#40973)

This reverts commit 7edd190, and fixes
a method type signature error, which caused TTY to get shutdown.
  • Loading branch information
vtjnash committed Sep 1, 2021
1 parent 4598966 commit 3aada59
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 37 deletions.
1 change: 1 addition & 0 deletions base/cmd.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
3 changes: 2 additions & 1 deletion base/filesystem.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
69 changes: 33 additions & 36 deletions base/process.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand Down
2 changes: 2 additions & 0 deletions base/stream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/spawn.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down

2 comments on commit 3aada59

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

Please sign in to comment.