[QUIC] Rewrite Open(Uni|Bidi)rectionalStream to leverage async stream opening

[CarnaViire]

Our current API for opening a QuicStream is synchronous and will throw if stream limit is reached. Waiting for available streams was intended to be a loop of checking for available stream count:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs

Lines 183 to 200 in c9f7f73

	while (true)
	{
	lock (SyncObj)
	{
	if (_connection == null)
	{
	break;
	}
	if (_connection.GetRemoteAvailableBidirectionalStreamCount() > 0)
	{
	quicStream = _connection.OpenBidirectionalStream();
	requestStream = new Http3RequestStream(request, this, quicStream);
	_activeRequests.Add(quicStream, requestStream);
	break;
	}
	waitTask = _connection.WaitForAvailableBidirectionalStreamsAsync(cancellationToken);

This proved to be highly inefficient - adding this loop around the opening eats up 80% of perf even when there's no actual need to wait.

MsQuic has it's own waiting logic implemented, and opening a stream can be asynchronous. As soon as stream is available, START_COMPLETE event will come to the stream. This can be done by passing ASYNC flag instead of FAIL_BLOCKED to StreamStart.

We need to:

• Change Open(Uni|Bidi)rectionalStream to Open(Uni|Bidi)rectionalStreamAsync, that will create the stream and then wait for the stream to be started
• We would need additional API on QuicStream; most possibly add "StartAsync" that will contain StreamStart call moved from .ctor and then wait on TCS for the event
• Remove wait for available streams loop from H3 (the code linked above)

Some thoughts:

• If cancellation occurs, if stream is already created but not started, it needs to be closed and disposed.
• Need to consider all places where TCS on opening a stream should be set to canceled (connection close, dispose etc)
• How dispose should work if stream is not started? Consider race conditions
• Do we need state checks (throwing on Send if stream is not started yet? etc)

Note: since msquic 2.0 I believe there will be no ASYNC flag anymore because async behavior is the default. (NONE flag)
Note 2: this is only for outbound streams. Inbound streams are already started when accepted.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Our current API for opening a QuicStream is synchronous and will throw if stream limit is reached. Waiting for available streams was intended to be a loop of checking for available stream count:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs

Lines 183 to 200 in c9f7f73

	while (true)
	{
	lock (SyncObj)
	{
	if (_connection == null)
	{
	break;
	}
	if (_connection.GetRemoteAvailableBidirectionalStreamCount() > 0)
	{
	quicStream = _connection.OpenBidirectionalStream();
	requestStream = new Http3RequestStream(request, this, quicStream);
	_activeRequests.Add(quicStream, requestStream);
	break;
	}
	waitTask = _connection.WaitForAvailableBidirectionalStreamsAsync(cancellationToken);

This proved to be highly inefficient - adding this loop around the opening eats up 80% of perf even when there's no actual need to wait.

MsQuic has it's own waiting logic implemented, and opening a stream can be asynchronous. As soon as stream is available, START_COMPLETE event will come to the stream. This can be done by passing ASYNC flag instead of FAIL_BLOCKED to StreamStart.

We need to:

• Change Open(Uni|Bidi)rectionalStream to Open(Uni|Bidi)rectionalStreamAsync, that will create the stream and then wait for the stream to be started
• We would need additional API on QuicStream; most possibly add "StartAsync" that will contain StreamStart call moved from .ctor and then wait on TCS for the event
• Remove wait for available streams loop from H3 (the code linked above)

Some thoughts:

• If cancellation occurs, if stream is already created but not started, it needs to be closed and disposed.
• Need to consider all places where TCS on opening a stream should be set to canceled (connection close, dispose etc)
• How dispose should work if stream is not started? Consider race conditions
• Do we need state checks (throwing on Send if stream is not started yet? etc)

Note: since msquic 2.0 I believe there will be no ASYNC flag anymore because async behavior is the default. (NONE flag)
Note 2: this is only for outbound streams. Inbound streams are already started when accepted.

Author:	CarnaViire
Assignees:	-
Labels:	untriaged, area-System.Net.Quic
Milestone:	-

[ManickaP]

I'm disabling 2 tests against this issue, because Open...directionalStream doesn't throw at the moment (the result gets returned in the callback).

[CarnaViire]

@JamesNK @Tratcher

We've just merged an API change for quic stream opening ( Open*directionalStream -> await Open*directionalStreamAsync )
I don't know if you've heard about the change before, if not, sorry for not letting you know sooner.

I believe you would need changes in opening control streams in Kestrel (I guess here https://github.com/dotnet/aspnetcore/blob/main/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionContext.cs#L192-L207). The change should be super easy, but let us know if you need our help