Skip to content
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

add stream shutdown and support half-duplex operation #40783

Merged
merged 1 commit into from
Jul 29, 2021
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented May 11, 2021

Fixes one of the issues mentioned in #24526

@vtjnash vtjnash added the io Involving the I/O subsystem: libuv, read, write, etc. label May 11, 2021
@vtjnash vtjnash requested a review from Keno May 11, 2021 01:11
@vtjnash
Copy link
Member Author

vtjnash commented May 11, 2021

required for #39027

@vtjnash
Copy link
Member Author

vtjnash commented May 11, 2021

CI failures are waiting for libuv/libuv#3006

base/io.jl Show resolved Hide resolved
@vtjnash vtjnash force-pushed the jn/shutdown branch 2 times, most recently from 6a98d0b to 5d9c442 Compare July 28, 2021 20:04
@vtjnash vtjnash merged commit 6c4f216 into master Jul 29, 2021
@vtjnash vtjnash deleted the jn/shutdown branch July 29, 2021 16:05
ararslan added a commit that referenced this pull request Aug 6, 2021
DilumAluthge pushed a commit that referenced this pull request Aug 6, 2021
DilumAluthge pushed a commit that referenced this pull request Aug 7, 2021
DilumAluthge pushed a commit that referenced this pull request Aug 7, 2021
DilumAluthge added a commit that referenced this pull request Aug 9, 2021
DilumAluthge added a commit that referenced this pull request Aug 11, 2021
DilumAluthge pushed a commit that referenced this pull request Aug 11, 2021
DilumAluthge added a commit that referenced this pull request Aug 19, 2021
@c42f
Copy link
Member

c42f commented Aug 25, 2021

I like everything about this PR except for the newly exported name shutdown().

Do we really want to export such a generic verb with such a relatively obscure meaning? (Yes, it maps to the names used in the underlying system calls, but IMO that's the only thing it's got going for it.)

In #24526 it was suggested that closewrite() might be used which seems far more obvious.

@vtjnash
Copy link
Member Author

vtjnash commented Aug 25, 2021

Ah, I didn't see discussion of names. I am okay with it, since it dispatches on being an IO object

@c42f
Copy link
Member

c42f commented Aug 25, 2021

I am okay with it, since it dispatches on being an IO object

To me this seems like close() but for the writer side? So closewrite() is a completely self-evident name if someone knows what close() does? Whereas shutdown() has no analogy, except for the small number of Juila programmers who also know about man 2 shutdown.

The fact that it only applies to IO abstractions is kind of the problem — other libraries should not really overload Base.shutdown() for unrelated functionality... that would be mixing semantics for the same verb.

Is closewrite() a correct renaming? I'm happy to make a PR.

@vtjnash
Copy link
Member Author

vtjnash commented Aug 25, 2021

Yeah, there is also a closeread option, but generally seems pretty useless, and libuv doesn't expose it.

@notinaboat
Copy link
Contributor

@c42f
Copy link
Member

c42f commented Aug 25, 2021

True. In HTTP, closeread(::Transaction) indicates that a transaction is done reading which unblocks the next transaction and enables it to start using the underlying TCP socket.

DilumAluthge added a commit that referenced this pull request Aug 25, 2021
DilumAluthge added a commit that referenced this pull request Aug 25, 2021
vtjnash added a commit that referenced this pull request Aug 25, 2021
vtjnash added a commit that referenced this pull request Aug 26, 2021
A stream can continue to be read after closewrite,
but cannot continue to be written to after seeing EOF.

Replaces #42004
Replaces #41983
Fixes #41942
Refs #40783
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
A stream can continue to be read after closewrite,
but cannot continue to be written to after seeing EOF.

Replaces JuliaLang#42004
Replaces JuliaLang#41983
Fixes JuliaLang#41942
Refs JuliaLang#40783
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
A stream can continue to be read after closewrite,
but cannot continue to be written to after seeing EOF.

Replaces JuliaLang#42004
Replaces JuliaLang#41983
Fixes JuliaLang#41942
Refs JuliaLang#40783
Keno pushed a commit that referenced this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Involving the I/O subsystem: libuv, read, write, etc. sockets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants