-
-
Notifications
You must be signed in to change notification settings - Fork 351
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
When building Channels on top of a bidirectional Stream, how should we handle closure? #895
Comments
Ah ha! You have discovered why v0.2.0 was delayed so long :-). I struggled this question a lot I was designing the abstract interfaces, and I think Trio's approach is the best available, but it does have trade-offs. So, the reason we have the hierarchy A We already have a way to describe an object that really is an independent (Interesting trivia: in the asyncio streams interface, if you have a bidi stream it's always represented as two objects, a Anyway... that's all background, I think, and the real question here is what to do with channels-on-top-of-streams. It's a really interesting question! Most protocols you build on top of streams are either intrinsically unidirectional or intrinsically bidirectional... channels are an interesting example where you have the same set of unidirectional/bidirectional options at the public API as you do on the transport layer, so you have to figure out how the sort-of-but-not-entirely-separate semantics of stream relate to that. Given the annoying fact that common transports don't have unidirectional close, it's not actually obvious to me that the best way to build a bidi I feel like what you might actually want to do is implement a single bidi protocol over a bidi
A piece of hard-won meta-advice: remember that the goal isn't to provide every mathematically plausible building block and allow fully arbitrary mixing-and-matching (as tempting as that is!); it's to provide a usable set of tools that people can actually understand and that are well-matched to the problems they encounter in practice. Does anyone actually need to multiplex a fully-independent |
Looked at this again today while writing #1110.... is there anything still to discuss here or can it be closed? |
I came across a slightly tricky implication of the structure of Trio's stream ABCs today.
Suppose I'm implementing a channel that sends objects over a byte stream. Since I'd like to be able to use it on a unidirectional transport, for maximum flexibility I'll define
StreamSendChannel
to wrap aSendStream
, andStreamReceiveChannel
to wrap aReceiveStream
. All good so far.Now I want to use these stream channels to send objects in both directions on a bidirectional transport that I have a
Stream
for. The subtyping relationships say anyStream
is-aSendStream
and also is-aReceiveStream
, so it looks like I can do this by wrapping the sameStream
in two channels, one for sending and one for receiving. This works fine as long as the channel stays open, but since the two channels are independent, they can close independently, and then they clash over the calls they want to make to the stream'saclose()
. The problem is thatStream.aclose()
doesn't start meaning onlySendStream.aclose()
if you interpret theStream
as aSendStream
-- it closes the receive side too.I think this might be less confusing if
Stream
did not inheritSendStream
andReceiveStream
. It could still define the same methods, and we could provide a helper for turning aStream
into aSendStream
plus aReceiveStream
, that encapsulates the logic of:aclose()
callssend_eof()
if impementedaclose()
callsshutdown_read()
if implemented (Add send_eof to SSLStream and get rid of HalfCloseableStream #823 (comment))aclose()
only called when both send and receive sides have been closedThis helper doesn't necessarily need to be provided by trio, and would be useful even if we don't change the inheritance hierarchy. I think it's probably not worth including in trio unless we change the inheritance hierarchy, so this issue is mostly about that question.
The text was updated successfully, but these errors were encountered: