Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 29 additions & 49 deletions base/stream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -665,75 +665,55 @@ end
function uv_readcb(handle::Ptr{Cvoid}, nread::Cssize_t, buf::Ptr{Cvoid})
stream_unknown_type = @handle_as handle LibuvStream
nrequested = ccall(:jl_uv_buf_len, Csize_t, (Ptr{Cvoid},), buf)

function readcb_specialized(stream::LibuvStream, nread::Int, nrequested::UInt)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Enabling "Hide whitespace" in the GitHub diff viewer, it seems to me that the changes to this function amount to:

  1. wrapping most of the code into a try/finally/end with unlock in the finalize block -- that seems plausible;
  2. inserting some empty lines, wrapping an overly long line -- I am neutral on this;
  3. removing or reducing a lot of comments -- that part seems less plausible: why do we want to remove comments or reduce them? Were they incorrect?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hey there,
Thank you for pointing out the mistake!
I am new to this whole open source contribution thing...
And about the comment part I never really use comments that's wrong on my part ( i know its a good practice to do so,
will try to keep that in mind as I move forward! )

lock(stream.cond)
if nread < 0
if nread == UV_ENOBUFS && nrequested == 0
# remind the client that stream.buffer is full
notify(stream.cond)
elseif nread == UV_EOF # libuv called uv_stop_reading already
if stream.status != StatusClosing
stream.status = StatusEOF
try
if nread < 0
if nread == UV_ENOBUFS && nrequested == 0
# remind the client that stream.buffer is full
notify(stream.cond)
if stream isa TTY
# stream can still be used by reseteof (or possibly write)
elseif !(stream isa PipeEndpoint) && ccall(:uv_is_writable, Cint, (Ptr{Cvoid},), stream.handle) != 0
# stream can still be used by write
else
# underlying stream is no longer useful: begin finalization
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), stream.handle)
stream.status = StatusClosing
elseif nread == UV_EOF
if stream.status != StatusClosing
stream.status = StatusEOF
notify(stream.cond)

if stream isa TTY
# still usable
elseif !(stream isa PipeEndpoint) &&
ccall(:uv_is_writable, Cint, (Ptr{Cvoid},), stream.handle) != 0
# still writable
else
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), stream.handle)
stream.status = StatusClosing
end
end
else
stream.readerror = _UVError("read", nread)
notify(stream.cond)
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), stream.handle)
stream.status = StatusClosing
end
else
stream.readerror = _UVError("read", nread)
notify_filled(stream.buffer, nread)
notify(stream.cond)
# This is a fatal connection error
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), stream.handle)
stream.status = StatusClosing
end
else
notify_filled(stream.buffer, nread)
notify(stream.cond)
finally
unlock(stream.cond)
end
unlock(stream.cond)

# Stop background reading when
# 1) there's nobody paying attention to the data we are reading
# 2) we have accumulated a lot of unread data OR
# 3) we have an alternate buffer that has reached its limit.
if stream.status == StatusPaused ||
(stream.status == StatusActive &&
((bytesavailable(stream.buffer) >= stream.throttle) ||
(bytesavailable(stream.buffer) >= stream.buffer.maxsize)))
# save cycles by stopping kernel notifications from arriving
ccall(:uv_read_stop, Cint, (Ptr{Cvoid},), stream)
stream.status = StatusOpen
end
nothing
end
readcb_specialized(stream_unknown_type, Int(nread), UInt(nrequested))
nothing
end

function reseteof(x::TTY)
iolock_begin()
if x.status == StatusEOF
x.status = StatusOpen
nothing
end
iolock_end()
Comment on lines -719 to -724
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is reseteof(x::TTY) being removed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I mistakenly removed the TTY fucntion
This will probably brake the interactive input...
Will add in the next commit.
Again thank you for pointing it out!

nothing
end

function _uv_hook_close(uv::Union{LibuvStream, LibuvServer})
lock(uv.cond)
try
uv.status = StatusClosed
# notify any listeners that exist on this libuv stream type
notify(uv.cond)
finally
unlock(uv.cond)
end
readcb_specialized(stream_unknown_type, Int(nread), UInt(nrequested))
nothing
end

Expand Down
32 changes: 31 additions & 1 deletion stdlib/Sockets/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ defaultport = rand(2000:4000)
wait(tsk)
end

mktempdir() do tmpdir
mktempdir() do tmpdir
socketname = Sys.iswindows() ? ("\\\\.\\pipe\\uv-test-" * randstring(6)) : joinpath(tmpdir, "socket")
local nconn = 0
srv = listen(socketname)
Expand All @@ -192,8 +192,38 @@ defaultport = rand(2000:4000)
wait(t)
@test read(conn, String) == ""
end

@static if !Sys.iswindows()
@testset "Unix domain socket half-close preserves writeability" begin
dir = mktempdir()
sock = joinpath(dir, "halfclose.sock")

server = listen(sock)

t = Threads.@spawn begin
c = accept(server)
read(c, String) # peer shutdown(SHUT_WR)
write(c, "pong\n") # MUST still succeed
flush(c)
close(c)
close(server)
:ok
end

out = read(pipeline(
`socat -t 1 - UNIX-CONNECT:$sock`,
stdin = IOBuffer("ping\n"),
), String)

srv = fetch(t)

@test out == "pong\n"
@test srv == :ok
end
end
end


@testset "getsockname errors" begin
sock = TCPSocket()
serv = Sockets.TCPServer()
Expand Down