Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

OpenStream needs to take a context #86

Closed
Stebalien opened this issue Apr 11, 2019 · 2 comments · Fixed by #172
Closed

OpenStream needs to take a context #86

Stebalien opened this issue Apr 11, 2019 · 2 comments · Fixed by #172

Comments

@Stebalien
Copy link
Member

This could block on some systems that restrict the number of open streams.

@raulk
Copy link
Member

raulk commented Apr 22, 2019

To provide some colour on this: yamux and mplex allow unlimited streams (well, bounded by the int length of the stream index) but we're not always upgrading connections via those multiplexers.

I guess the original authors of the API go-stream-muxer API assumed that opening a stream is cheap and non-blocking. However, with transports like QUIC, where multiplexing is native, YMMV.

Ways forward

I can think of two:

  1. Change the method signature in the interface, and change the mux adaptors to conform. This implies breakage.
  2. Keep the interface intact and add a new interface, adapting the call sites to type cast and pass the context in accordingly.

I'd prefer 1 -- with gomod the impact should be limited.


Do we need to pass in a context to AcceptStream too?

@Stebalien
Copy link
Member Author

Do we need to pass in a context to AcceptStream too?

Probably not. AcceptStream is an analog of Listener.Accept and can be interrupted by closing the listener.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants