quinn icon indicating copy to clipboard operation
quinn copied to clipboard

StreamIds are allocated in the order in which they are polled, rather than the order of `open_uni`/`open_bi`

Open alecmocatta opened this issue 5 years ago • 7 comments

The docs specify incoming streams as being "in the order they were opened". Intuitively I'd expect this to be the order in which open_uni/open_bi were called, but it's actually the order in which they were polled.

Ordering according to open_* is more explicit, the programmer has greater control over it (e.g. join_all doesn't specify in which order it polls futures), plus I believe it to be less surprising. It might be tricky due to flow control, but it seems possible. Is it worth resolving?

alecmocatta avatar Mar 06 '20 11:03 alecmocatta

The general rule in rust is that futures do nothing until they're polled. I agree that the resulting behavior could catch someone by surprise, though I expect the overwhelming majority of cases where someone cares about stream ordering will involve serial awaiting.

A fix could be implemented by modifying quinn_proto::Connection::open to always successfully return a new ID, and introduce a separate mechanism to poll whether a stream is actually open yet. The open futures would then allocate an ID synchronously, and wait for it to become open before yielding the resulting handle.

Ralith avatar Mar 06 '20 18:03 Ralith

Would that allow opening streams (as the peer sees it) in a different order than what IDs are allocated? I worry that that might also be confusing in different ways.

djc avatar Mar 06 '20 18:03 djc

Remotely-initiated streams are always observed opening strictly in ID order. The "poll whether a stream is open" mechanism would be nothing more than stream_id.index() < self.streams.max[stream_id.dir()].

Ralith avatar Mar 06 '20 18:03 Ralith

Remotely-initiated streams are always observed opening strictly in ID order.

Is that true for other implementations?

djc avatar Mar 06 '20 18:03 djc

Yes, it's a property of the spec. Even if you receive stream frames out of order, receiving a higher-numbered stream always implicitly opens the lower-numbered ones first:

Before a stream is created, all streams of the same type with lower-numbered stream IDs MUST be created. This ensures that the creation order for streams is consistent on both endpoints.

Ralith avatar Mar 06 '20 19:03 Ralith

This issue is old. Is it still valid?

I've checked the docs for quinn_proto::Connection::set_max_concurrent_streams and it seems that the code is already open all ids following the stream_id.index() < self.streams.max[stream_id.dir()] condition.

lmpn avatar Nov 06 '23 21:11 lmpn

Yes, this issue is current. The problem it describes is that outgoing stream IDs are assigned based on the order in which the stream opening futures are first polled, which could be surprising in certain rare cases. Typically people await the open futures serially, so it doesn't come up.

Ralith avatar Nov 06 '23 23:11 Ralith