From 7d9bfb73361def90f7b3ec58d68e9fe366548abb Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 11 Jun 2020 03:30:08 -0500 Subject: [PATCH 1/3] Formal specification of field types for abstract IO types Most concrete IO types declare that a subset of their fields are other abstract IO types. As a result, if `obj1::IO`, and `obj2 = obj1.fieldwithabstracttype`, then inference has no way of knowing what type `obj2.status` returns. When `obj2.status` is used as an argument for `Base.somefunction`, this leaves such code vulnerable to invalidation via package specializations of `Base.somefunction` even when in practice there is no risk that any IO code will call the new package-supplied method. This provides a more formal interface for three non-exported abstract types: `LibuvStream`, `LibuvServer`, and `AbstractPipe`. --- base/io.jl | 21 +++++++++++++++++- base/stream.jl | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/base/io.jl b/base/io.jl index 61c686a38529b..476af15e58368 100644 --- a/base/io.jl +++ b/base/io.jl @@ -333,8 +333,27 @@ function open(f::Function, args...; kwargs...) end end -# Generic wrappers around other IO objects +""" + AbstractPipe + +`AbstractPipe` is the abstract supertype for IO pipes that provide for communication between processes. + +If `pipe isa AbstractPipe`, it must obey the following interface: + +- `pipe.in` or `pipe.in_stream`, if present, must be of type `IO` and be used to provide input to the pipe +- `pipe.out` or `pipe.out_stream`, if present, must be of type `IO` and be used for output from the pipe +- `pipe.err` or `pipe.err_stream`, if present, must be of type `IO` and be used for writing errors from the pipe +""" abstract type AbstractPipe <: IO end + +function getproperty(pipe::AbstractPipe, name::Symbol) + if name === :in || name === :in_stream || name === :out || name === :out_stream || + name === :err || name === :err_stream + return getfield(pipe, name)::IO + end + return getfield(pipe, name) +end + function pipe_reader end function pipe_writer end diff --git a/base/stream.jl b/base/stream.jl index d5f3b05fc0327..f824334fb1418 100644 --- a/base/stream.jl +++ b/base/stream.jl @@ -13,9 +13,67 @@ end ## types ## abstract type IOServer end +""" + LibuvServer + +An abstract type for IOServers handled by libuv. + +If `server isa LibuvServer`, it must obey the following interface: + +- `server.handle` must be a `Ptr{Cvoid}` +- `server.status` must be an `Int` +- `server.cond` must be a `GenericCondition` +""" abstract type LibuvServer <: IOServer end + +function getproperty(server::LibuvServer, name::Symbol) + if name === :handle + return getfield(server, :handle)::Ptr{Cvoid} + elseif name === :status + return getfield(server, :status)::Int + elseif name === :cond + return getfield(server, :cond)::GenericCondition + else + return getfield(server, name) + end +end + +""" + LibuvStream + +An abstract type for IO streams handled by libuv. + +If`stream isa LibuvStream`, it must obey the following interface: + +- `stream.handle`, if present, must be a `Ptr{Cvoid}` +- `stream.status`, if present, must be an `Int` +- `stream.buffer`, if present, must be an `IOBuffer` +- `stream.sendbuf`, if present, must be a `Union{Nothing,IOBuffer}` +- `stream.cond`, if present, must be a `GenericCondition` +- `stream.lock`, if present, must be an `AbstractLock` +- `stream.throttle`, if present, must be an `Int` +""" abstract type LibuvStream <: IO end +function getproperty(stream::LibuvStream, name::Symbol) + if name === :handle + return getfield(stream, :handle)::Ptr{Cvoid} + elseif name === :status + return getfield(stream, :status)::Int + elseif name === :buffer + return getfield(stream, :buffer)::IOBuffer + elseif name === :sendbuf + return getfield(stream, :sendbuf)::Union{Nothing,IOBuffer} + elseif name === :cond + return getfield(stream, :cond)::GenericCondition + elseif name === :lock + return getfield(stream, :lock)::AbstractLock + elseif name === :throttle + return getfield(stream, :throttle)::Int + else + return getfield(stream, name) + end +end # IO # +- GenericIOBuffer{T<:AbstractArray{UInt8,1}} (not exported) From 6a69baad9370f3ebe3eb88a5ad642b3631c34294 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 11 Jun 2020 03:35:53 -0500 Subject: [PATCH 2/3] Remove obsolete field type assertions --- base/stream.jl | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/base/stream.jl b/base/stream.jl index f824334fb1418..6e9ef071b7f50 100644 --- a/base/stream.jl +++ b/base/stream.jl @@ -378,7 +378,7 @@ function isopen(x::Union{LibuvStream, LibuvServer}) if x.status == StatusUninit || x.status == StatusInit throw(ArgumentError("$x is not initialized")) end - return x.status::Int != StatusClosed && x.status::Int != StatusEOF + return x.status != StatusClosed && x.status != StatusEOF end function check_open(x::Union{LibuvStream, LibuvServer}) @@ -453,7 +453,7 @@ function close(stream::Union{LibuvStream, LibuvServer}) stream.status = StatusClosing elseif isopen(stream) || stream.status == StatusEOF should_wait = uv_handle_data(stream) != C_NULL - if stream.status::Int != StatusClosing + if stream.status != StatusClosing ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), stream.handle) stream.status = StatusClosing end @@ -468,7 +468,7 @@ function uvfinalize(uv::Union{LibuvStream, LibuvServer}) iolock_begin() if uv.handle != C_NULL disassociate_julia_struct(uv.handle) # not going to call the usual close hooks - if uv.status::Int != StatusUninit + if uv.status != StatusUninit close(uv) else Libc.free(uv.handle) @@ -582,7 +582,7 @@ function uv_alloc_buf(handle::Ptr{Cvoid}, size::Csize_t, buf::Ptr{Cvoid}) stream = unsafe_pointer_to_objref(hd)::LibuvStream local data::Ptr{Cvoid}, newsize::Csize_t - if stream.status::Int != StatusActive + if stream.status != StatusActive data = C_NULL newsize = 0 else @@ -615,7 +615,7 @@ function uv_readcb(handle::Ptr{Cvoid}, nread::Cssize_t, buf::Ptr{Cvoid}) if isa(stream, TTY) stream.status = StatusEOF # libuv called uv_stop_reading already notify(stream.cond) - elseif stream.status::Int != StatusClosing + elseif stream.status != StatusClosing # begin shutdown of the stream ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), stream.handle) stream.status = StatusClosing @@ -725,7 +725,7 @@ show(io::IO, stream::Pipe) = print(io, function open_pipe!(p::PipeEndpoint, handle::OS_HANDLE) iolock_begin() - if p.status::Int != StatusInit + if p.status != StatusInit error("pipe is already in use or has been closed") end err = ccall(:uv_pipe_open, Int32, (Ptr{Cvoid}, OS_HANDLE), p.handle, handle) @@ -1119,7 +1119,7 @@ _fd(x::Union{OS_HANDLE, RawFD}) = x function _fd(x::Union{LibuvStream, LibuvServer}) fd = Ref{OS_HANDLE}(INVALID_OS_HANDLE) - if x.status::Int != StatusUninit && x.status::Int != StatusClosed + if x.status != StatusUninit && x.status != StatusClosed err = ccall(:uv_fileno, Int32, (Ptr{Cvoid}, Ptr{OS_HANDLE}), x.handle, fd) # handle errors by returning INVALID_OS_HANDLE end From 44f88a87be34e928528bb9ba0ee6a3f33f4edc79 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Wed, 17 Jun 2020 03:44:39 -0500 Subject: [PATCH 3/3] Assert type for `displaysize` --- base/show.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/show.jl b/base/show.jl index 300acd186e14b..e77e2b9735317 100644 --- a/base/show.jl +++ b/base/show.jl @@ -340,7 +340,7 @@ getindex(io::IO, key) = throw(KeyError(key)) get(io::IOContext, key, default) = get(io.dict, key, default) get(io::IO, key, default) = default -displaysize(io::IOContext) = haskey(io, :displaysize) ? io[:displaysize] : displaysize(io.io) +displaysize(io::IOContext) = haskey(io, :displaysize) ? io[:displaysize]::Tuple{Int,Int} : displaysize(io.io) show_circular(io::IO, @nospecialize(x)) = false function show_circular(io::IOContext, @nospecialize(x))