-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Clarify documentation around close
, flush
, and isopen
#58020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -9,7 +9,8 @@ Create a async condition that wakes up tasks waiting for it | |||||
(by calling [`wait`](@ref) on the object) | ||||||
when notified from C by a call to `uv_async_send`. | ||||||
Waiting tasks are woken with an error when the object is closed (by [`close`](@ref)). | ||||||
Use [`isopen`](@ref) to check whether it is still active. | ||||||
Use [`isopen`](@ref) to check whether it is still active. A closed condition will | ||||||
not wake up tasks. | ||||||
|
||||||
This provides an implicit acquire & release memory ordering between the sending and waiting threads. | ||||||
""" | ||||||
|
@@ -74,7 +75,8 @@ Create a timer that wakes up tasks waiting for it (by calling [`wait`](@ref) on | |||||
Waiting tasks are woken after an initial delay of at least `delay` seconds, and then repeating after | ||||||
at least `interval` seconds again elapse. If `interval` is equal to `0`, the timer is only triggered | ||||||
once. When the timer is closed (by [`close`](@ref)) waiting tasks are woken with an error. Use | ||||||
[`isopen`](@ref) to check whether a timer is still active. Use `t.timeout` and `t.interval` to read | ||||||
[`isopen`](@ref) to check whether a timer is still active. A closed timer will not fire. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
Use `t.timeout` and `t.interval` to read | ||||||
the setup conditions of a `Timer` `t`. | ||||||
|
||||||
```julia-repl | ||||||
|
@@ -206,6 +208,14 @@ end | |||||
|
||||||
isopen(t::Union{Timer, AsyncCondition}) = @atomic :acquire t.isopen | ||||||
|
||||||
""" | ||||||
close(t::Union{Timer, AsyncCondition}) | ||||||
|
||||||
Close an object `t`. Once a timer or condition is closed, it will not produce | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Continuing from above, how about
Suggested change
|
||||||
a new event. | ||||||
|
||||||
See also: [`isopen`](@ref) | ||||||
""" | ||||||
function close(t::Union{Timer, AsyncCondition}) | ||||||
t.handle == C_NULL && !t.isopen && return # short-circuit path, :monotonic | ||||||
iolock_begin() | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -41,11 +41,9 @@ buffer_writes(x::IO, bufsize=SZ_UNBUFFERED_IO) = x | |||||
""" | ||||||
isopen(object)::Bool | ||||||
|
||||||
Determine whether an object - such as a stream or timer | ||||||
-- is not yet closed. Once an object is closed, it will never produce a new event. | ||||||
However, since a closed stream may still have data to read in its buffer, | ||||||
use [`eof`](@ref) to check for the ability to read data. | ||||||
Use the `FileWatching` package to be notified when a stream might be writable or readable. | ||||||
Determine whether an object, such as an IO or timer, is not yet closed. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
See also: [`close`](@ref) | ||||||
|
||||||
# Examples | ||||||
```jldoctest | ||||||
|
@@ -63,11 +61,27 @@ false | |||||
function isopen end | ||||||
|
||||||
""" | ||||||
close(stream) | ||||||
close(io::IO) | ||||||
|
||||||
Close `io`. Performs a [`flush`](@ref) first. | ||||||
|
||||||
Closing an IO signals that its underlying resources (OS handle, network | ||||||
connections, etc) should be destroyed. | ||||||
A closed IO is in an undefined state and should not be written to or read from. | ||||||
When attempting to do so, the IO may throw an exception, continue to behave | ||||||
normally, or read/write zero bytes, depending on the implementation. | ||||||
However, implementations should make sure that reading to or writing from a | ||||||
closed IO does not cause undefined behaviour. | ||||||
|
||||||
Close an I/O stream. Performs a [`flush`](@ref) first. | ||||||
This function is generically defined to only `flush` the io. That allows | ||||||
wrapping IOs to close its underlying IO. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
See also: [`isopen`](@ref) | ||||||
""" | ||||||
function close end | ||||||
function close(io::IO) | ||||||
flush(io) | ||||||
nothing | ||||||
end | ||||||
Comment on lines
+81
to
+84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this a change in behavior and thus goes beyond what the title of this PR suggests? I feel a bit uncomfortable with this -- I'd immediately agree to merge the docstring changes, but I don't feel qualified to judge whether this change is OK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. The rest seems good |
||||||
|
||||||
""" | ||||||
closewrite(stream) | ||||||
|
@@ -97,9 +111,11 @@ julia> read(io, String) | |||||
function closewrite end | ||||||
|
||||||
""" | ||||||
flush(stream) | ||||||
flush(io::IO) | ||||||
|
||||||
Commit all currently buffered writes to the given stream. | ||||||
Commit all currently buffered writes to the given io. | ||||||
This has a default implementation `flush(::IO) = nothing`, so may be called | ||||||
in generic IO code. | ||||||
""" | ||||||
function flush end | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this would be better? Yes, the function is called
isopen
but the text says "active", not "open?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I see elsewhere the text already talks about "closed". I find this use of two different terms meaning the same thing adds a layer of potential confusion. So perhaps like this: