Skip to content

Conversation

@mkitti
Copy link
Contributor

@mkitti mkitti commented Feb 22, 2024

In #53392, I noticed a missing API call to check if a stream had a state of Base.StatusClosing.

A stream acquires a state of Base.StatusClosing after one uses close(stream).
A stream acquires the state of Base.StatusClosed once libuv calls back into Julia to confirm
the closed state.

To avoid the implementation details of how an IO stream's state from leaking out of base, I
propose we add Base.isclosing and Base.isclosed methods.

@vtjnash
Copy link
Member

vtjnash commented Feb 22, 2024

That is not what is happening in #53392 though, which is a definite libuv bug which will not be in any way benefited by these methods, which appears to be caused by a known limitation with epoll's implementation in the linux kernel (we can work around this in libuv as the epoll implementation issue is documented)

@quinnj
Copy link
Member

quinnj commented Feb 22, 2024

isn't isclosed just !isopen?

@mkitti
Copy link
Contributor Author

mkitti commented Feb 22, 2024

@vtjnash I agree that this has no direct impact on the issue uncovered by JuliaDebug/Cthulhu.jl#541. The primary cause is within libuv as you describe.

Independent of that issue it is not clear to me there why the REPL should fail in that finally block when the input stream of the terminal is closed. That finally block exists solely to reset the raw mode of the terminal. If the input stream of the terminal is closed or closing, there is no need to reset the raw state. Thus we really do not need to throw an error in that case.

finally # always disable raw mode
if raw_mode_enabled
!suppress_output && print(term.out_stream, "\x1b[?25h") # unhide cursor
try
# raw! calls check_open which can throw
REPL.Terminals.raw!(term, false)
catch err
if err !== Base.IOError("stream is closed or unusable", 0)
rethrow()
end
end
end
end

In #53392 I proposed that we catch the specific error thrown by check_open and "handle" it since there is no need to reset the raw state if the terminal is closed. Furthermore, there is no other API available to anticipate the error without examining the status field directly or perhaps by using uv_status_string. The alternative to catching the error I presented was as follows.

in_stream = pipe_reader(term)
if isopen(in_stream) && in_stream.status != Base.StatusClosing
    # call method that throws if in_stream is closed or closing.
end

Rather than access the field directly, I thought there should be an API such that one could do the following.

if isopen(in_stream) && !isclosing(in_stream)
    # call method that throws if in_stream is closed or closing.
end

isn't isclosed just !isopen?

@quinnj Here is the definition of isopen. It requires not only the absence of the closed state, but also that the stream has been initialized.

julia/base/stream.jl

Lines 379 to 384 in 02699bb

function isopen(x::Union{LibuvStream, LibuvServer})
if x.status == StatusUninit || x.status == StatusInit || x.handle === C_NULL
throw(ArgumentError("$x is not initialized"))
end
return x.status != StatusClosed
end

Checking for the closed state seems much simpler. We only need to do x.status == StatusClosed. If that is true we know that that status is neither StatusUninit or StatusInit. We can discuss further if we need to check that the handle is C_NULL, but I suspect that the handle state may also not be relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants