[API Proposal]: [QUIC] QuicStream

[ManickaP]

Background and motivation

API design for exposing QuicStream and related classes to the public.

The API shape is mainly defined by Stream from which it derives. QuicStream has some additional APIs specific for QUIC, but it still has a goal of QuicStream to be usable as an ordinary stream, e.g.: allow usage like this:

await using var stream = ...; // Get QuicStream
await stream.WriteAsync(...);
await stream.ReadAsync(...);
// Dispose by await using

Albeit better results can be achieved via the additional API. For example from the previous code snippet, the stream didn't complete writes until DisposeAsync got called. Thus the peer might still be expecting incoming data and never send any itself. All of this depends on the specific protocol and the contract between the peers. For instance in HTTP/3 case, we wrap QuicStream into HTTP/3 specific stream taking care of all the proprieties with completing the write side of the stream, which is then handed out in the response.

This API proposal concentrates on the additional, QUIC specific APIs.

Related issues:

• QUIC: Revisit QuicStream.ReadsComplete API #57780
• [QUIC] Design of Shutdown for QuicStreams in System.Net.Quic #756
• Feature Request: Add ability to explicitly send EOF for connected Streams #43290
• [QUIC] Introduce properties to distiguish uni/bi directional streams #55816

API Proposal

namespace System.Net.Quic;

public sealed class QuicStream : Stream
{
    // Stream API implied, QUIC specifics follow:

    public long Id { get; } // https://www.rfc-editor.org/rfc/rfc9000.html#name-stream-types-and-identifier
    public QuicStreamType Type { get; }  // https://github.com/dotnet/runtime/issues/55816, not necessary per se, CanRead and CanWrite should suffice

    // In 6.0 we had bool ReadsCompleted (tailored for ASP.NET) that would get set only in graceful case. The task gives the ability to distinguish error cases and is analogous to WritesCompleted.
    public Task ReadsClosed { get; } // gets set when STREAM frame with FIN bit (=EOF, =ReadAsync returning 0) is received or when the peer aborts the sending side by sending RESET_STREAM frame. Inspired by channel - might be ValueTask.
    
    // In 6.0 we had a method Task WaitForWriteCompletionAsync(). We need a Task that is removed from the operation kicking the write completion but gets completed when the sending side gets closed (either by CompleteWrites, endStream=true, Abort(Write) or Abort(Read) from the peer).
    public Task WritesClosed { get; } // gets set when the peer acknowledges STREAM frame with FIN bit (=EOF) or RESET_STREAM frame or when the peer aborts the receiving side by sending STOP_SENDING frame. Inspired by channel - might be ValueTask.

    // In 6.0 we had separate methods AbortRead and AbortWrite. This allows aborting both directions at the same time. It can remain split, it's not strictly necessary to have it combined.
    public void Abort(QuicAbortDirection abortDirection, long errorCode); // abortively ends either sending or receiving or both sides of the stream, i.e.: RESET_STREAM frame or STOP_SENDING frame

    // In 6.0 we had void Shutdown(), it was really badly named. The new name comes from DuplexStream API review, where CompleteWrites was chosen,
    public void CompleteWrites(); // https://github.com/dotnet/runtime/issues/43290, gracefully ends the sending side, equivalent to WriteAsync with endStream set to true.

    // Overload with endStream.
    public ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, bool completeWrites, CancellationToken cancellationToken = default);
}

[Flags]
enum QuicAbortDirection
{
   Read = 1,
   Write = 2,
   Both = Read | Write
}

public enum QuicStreamType
{
    Unidirectional,
    Bidirectional
}

API Usage

Client usage:

// Consider connection from https://github.com/dotnet/runtime/issues/68902:
await using var stream = await connection.OpenStreamAsync(QuicStreamType.Bidirectional, cancellationToken);
// Send some data.
await stream.WriteAsync(data, cancellationToken);
await stream.WriteAsync(data, cancellationToken);

// End the sending-side.
await stream.WriteAsync(data, endStream: true, cancellationToken);
// or
await stream.WriteAsync(data, cancellationToken);
stream.CompleteWrites();

// Read data until the end of stream.
while (await stream.ReadAsync(buffer, cancellationToken) > 0)
{
    // Handle buffer data...
}

// DisposeAsync called by await using.

Server usage:

// Consider connection from https://github.com/dotnet/runtime/issues/68902:
await using var stream = await connection.AcceptStreamAsync(cancellationToken);

// Read the data.
while (stream.ReadAsync(buffer, cancellationToken) > 0)
{
    // Handle buffer data...

    // Peer send FIN flag with the last read.
    if (stream.ReadsCompleted.IsCompleted)
    {
        break;
    }
}

// Listen for Abort(Read) (STOP_SENDING) from the peer.
var writesCompletedTask = WritesCompletedAsync(stream);
async ValueTask WritesCompletedAsync(QuicStream stream)
{
    try
    {
        await stream.WritesCompleted;
    }
    catch (Exception ex)
    {
        // Abort the stream, peer send STOP_SENDING
    }
}

// DisposeAsync called by await using.

Alternative Designs

Read is finished

#57780
Currently we have a bool property, tailored to ASP.NET core needs, the property gets set only on graceful completion of the peer's sending side.

// Original. If it works... :)
public bool ReadsCompleted { get; }

// Proposal covers also abortive completions with details
public Task ReadsCompleted { get; } 

Sending EOF, getting notified about EOF

#43290
Currently we have:

// Gracefully ends the sending side, equivalent to endStream argument of WriteAsync
public void Shutdown();
// Doesn't kick off any operation leading to stream shutdown (for that Shutdown or Abort must be called). 
// Returns Task that gets completed when we gracefully end sending or we receive STOP_SENDING (i.e. peer called AbortRead).
public ValueTask WaitForWriteCompletionAsync(CancellationToken cancellationToken);

In #43290 (#43290 (comment)), this relevant part of API was approved:

// Shutdown renamed, in this proposal we have the same API.
public abstract void CompleteWrites();
// Task WritesCompleted property equivalent. Looks like async version of CompleteWrites but isn't, since it doesn't kick off any operation leading to stream sending completion.
public abstract ValueTask CompleteWritesAsync(CancellationToken cancellationToken = default);

Abortive completion

#756
Currently we have 2 methods:

public void AbortRead(long errorCode);
public void AbortWrite(long errorCode);

In #756 (#756 (comment)) we agreed on what this proposal has:

public void Abort(QuicAbortDirection abortDirection, long errorCode);

Stream type

#55816
We can use CanRead and CanWrite from stream as we do now, no need for any additional API. The only disadvantage is that they both return false after disposal, thus are not useful at that time. Whether that's a valid / expected usage is a different question.

Risks

As I'll state with all QUIC APIs. We might consider making all of these PreviewFeature. Not to deter user from using it, but to give us flexibility to tune the API shape based on customer feedback.
We don't have many users now and we're mostly making these APIs based on what Kestrel needs, our limited experience with System.Net.Quic and my meager experiments with other QUIC implementations.

[ghost]

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

Issue Details

---

Background and motivation

API design for exposing QuicStream and related classes to the public.

The API shape is mainly defined by Stream from which it derives. QuicStream has some additional APIs specific for QUIC, but it still has a goal of QuicStream to be usable as an ordinary stream, e.g.: allow usage like this:

await using var stream = ...; // Get QuicStream
await stream.WriteAsync(...);
await stream.ReadAsync(...);
// Dispose by await using

Albeit better results can be achieved via the additional API. For example from the previous code snippet, the stream didn't complete writes until DisposeAsync got called. Thus the peer might still be expecting incoming data and never send any itself. All of this depends on the specific protocol and the contract between the peers. For instance in HTTP/3 case, we wrap QuicStream into HTTP/3 specific stream taking care of all the proprieties with completing the write side of the stream, which is then handed out in the response.

This API proposal concentrates on the additional, QUIC specific APIs. The whole API shape is in details at the end.

API Proposal

public class QuicStream : Stream
{
    // Stream API implied, QUIC specifics follow:

    public long StreamId { get; } // https://www.rfc-editor.org/rfc/rfc9000.html#name-stream-types-and-identifier
    public StreamDirection { get; }  // https://github.com/dotnet/runtime/issues/55816
    public Task ReadsCompleted { get; } // gets set when peer sends STREAM frame with FIN bit (=EOF, =ReadAsync returning 0) or when peer aborts the sending side by sending RESET_STREAM frame
    public Task WritesCompleted { get; } // gets set our side sends STREAM frame with FIN bit (=EOF) or when peer aborts the receiving side by sending STOP_SENDING frame
    public void Abort(Read|Write flag, long errorCode); // abortively ends either sending ore receiving or both sides of the stream, i.e.: RESET_STREAM frame or STOP_SENDING frame
    public void CompleteWrites(); // https://github.com/dotnet/runtime/issues/43290, gracefully ends the sending side, equivalent to WriteAsync with endStream set to true

    // New overloads for WriteAsync, each of them introduces overload with bool endStream.
    // Used by ASP.NET core.
    public ValueTask WriteAsync(ReadOnlySequence<byte> buffers, CancellationToken cancellationToken);
    public ValueTask WriteAsync(ReadOnlySequence<byte> buffers, bool endStream, CancellationToken cancellationToken);
    // Used by HttpClient.
    public ValueTask WriteAsync(ReadOnlyMemory<ReadOnlyMemory<byte>> buffers, CancellationToken cancellationToken);
    public ValueTask WriteAsync(ReadOnlyMemory<ReadOnlyMemory<byte>> buffers, bool endStream, CancellationToken cancellationToken);
    // Overload with endStream for the one inherited from Stream.
    public ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, bool endStream, CancellationToken cancellationToken);

API Usage

TODO

Alternative Designs

No response

Risks

No response

Author:	ManickaP
Assignees:	ManickaP
Labels:	api-suggestion, area-System.Net.Quic
Milestone:	7.0.0

[terrajobst]

• StreamId
  • Consider renaming to Id
• ReadsCompleted and WritesCompleted is "inverted" as it refers to the state of the other side
• WriteAsync
  • The parameter cancellationToken should be defaulted
  • Avoid the readonly sequence overloads
  • PeerReadsCompleted or PeerWritesCompleted might be better
• The terminology of unidirectional is a bit confusing because it seems to imply writing

namespace System.Net.Quic;

public sealed class QuicStream : Stream
{
    public long StreamId { get; }
    public QuicStreamType StreamType { get; }
    public Task ReadsCompleted { get; }
    public Task WritesCompleted { get; }
    public void Abort(QuicAbortDirection abortDirection, long errorCode);
    public void CompleteWrites();
    public ValueTask WriteAsync(ReadOnlySequence<byte> buffers, CancellationToken cancellationToken);
    public ValueTask WriteAsync(ReadOnlySequence<byte> buffers, bool endStream, CancellationToken cancellationToken);
    public ValueTask WriteAsync(ReadOnlyMemory<ReadOnlyMemory<byte>> buffers, CancellationToken cancellationToken);
    public ValueTask WriteAsync(ReadOnlyMemory<ReadOnlyMemory<byte>> buffers, bool endStream, CancellationToken cancellationToken);
    public ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, bool endStream, CancellationToken cancellationToken);
}
[Flags]
public enum QuicAbortDirection
{
   Read = 1,
   Write = 2,
   Both = Read | Write
}
public public enum QuicStreamType
{
    Unidirectional,
    Bidirectional
}

[ManickaP]

API Review Comments

StreamId

• Consider renaming to Id
  • Changed in the proposal to Id as well as StreamType to Type

ReadsCompleted and WritesCompleted is "inverted" as it refers to the state of the other side

• PeerReadsCompleted or PeerWritesCompleted might be better
  • They are not only completed by the peer, there are also local events leading to the Task completion (successful and exceptional). We came up with alternative naming with the word Closed, expressing that that particular direction of the stream is closed and no longer available for usage. This was discussed with @stephentoub offline and we reached an agreement on this naming.
  • Changed in the proposal to ReadsClosed and WritesClosed.

WriteAsync

• The parameter cancellationToken should be defaulted
  • _ Changed in the proposal to be defaulted._
• Avoid the readonly sequence overloads
  • Overloads used by HttpClient (taking ReadOnlyMemory<ReadOnlyMemory<byte>>) were removed since the perf impact was not measurable when we benchmarked it.
  • Overloads used by ASP.NET Core will also be removed (Change QUIC transport to use multiple calls to QuicStream.WriteAsync aspnetcore#42464).
  • I have also spoken with @adamsitnik about scatter-gather API shape, and in case we decide to add some overload it should be: ValueTask WriteAsync(IReadOnlyList<ReadOnlyMemory<byte>> buffers, CancellationToken cancellationToken = default);
  • Removed from the proposal.

The terminology of unidirectional is a bit confusing because it seems to imply writing

• This is terminology/definition from the specification, there isn't much to do about it.
• Not changing the proposal.

Side notes

I also investigated behavior of Socket and its Dispose in case of an RST. The question was what happens when client sends data (they are buffered in the OS, not yet all sent and ACKed by the server) and disposes the connection, while the server resets the connection without reading all the data. Will the client learn that server hasn't consumed all the data? Will Dispose throw? The answer is NO. So in the light of this, our QuicStream.DisposeAsync will not throw even if the peer aborts the reading side while all the writes passed without an issue and we're closing the stream. Note that in this case WritesClosed will transition into failed state so the information will still be available to the user.

Testing code:

class Program
{
    static async Task Main(string[] args)
    {
        await Task.WhenAll(RunServer(), RunClient());
    }

    static readonly TaskCompletionSource<IPEndPoint> serverEndpoint = new TaskCompletionSource<IPEndPoint>();
    static readonly TaskCompletionSource clientWrite = new TaskCompletionSource();

    static async Task RunClient()
    {
        var socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
        socket.NoDelay = true;
        var endpoint = await serverEndpoint.Task;
        Console.WriteLine("Client connecting to: " + endpoint);
        socket.Connect(endpoint);
        Console.WriteLine("Client connected to: " + socket.RemoteEndPoint);
        var stream = new NetworkStream(socket, ownsSocket: true);
        for (int i = 0; i < 1_000; ++i)
        {
            await stream.WriteAsync(UTF8Encoding.UTF8.GetBytes("Ahoj"));
        }
        clientWrite.SetResult();
        /*try
        {
            // Read will throw "Connection reset by peer".
            await stream.ReadAsync(new byte[10]);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex);
        }*/
        socket.Shutdown(SocketShutdown.Send);
        stream.Dispose();
    }
    static async Task RunServer()
    {
        var listenSocket = new Socket(SocketType.Stream, ProtocolType.Tcp);
        listenSocket.Bind(new IPEndPoint(IPAddress.Loopback, 0));
        listenSocket.Listen();
        serverEndpoint.SetResult(listenSocket.LocalEndPoint as IPEndPoint);
        Console.WriteLine("Server listening on: " + listenSocket.LocalEndPoint);
        var socket = await listenSocket.AcceptAsync().ConfigureAwait(false);
        socket.NoDelay = true;
        // To force RST when calling Close.
        socket.LingerState = new LingerOption(true, 0);
        var stream = new NetworkStream(socket, ownsSocket: true);
        var buffer = new byte[100];
        int readBytes = 0;
        do
        {
            readBytes = await stream.ReadAsync(buffer);
            Console.WriteLine($"Server({readBytes}):" + UTF8Encoding.UTF8.GetString(buffer, 0, readBytes));
            await clientWrite.Task;
            socket.Close(0);
        } while (false);//(readBytes > 0);
    }
}

[terrajobst]

Video

• Looks good as proposed, given that it's in preview.

namespace System.Net.Quic;

public sealed class QuicStream : Stream
{
    public long Id { get; }
    public QuicStreamType Type { get; }
    public Task ReadsClosed { get; }
    public Task WritesClosed { get; }
    public void Abort(QuicAbortDirection abortDirection, long errorCode);
    public void CompleteWrites();
    public ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, bool completeWrites, CancellationToken cancellationToken = default);
}

[Flags]
public enum QuicAbortDirection
{
   Read = 1,
   Write = 2,
   Both = Read | Write
}

public enum QuicStreamType
{
    Unidirectional,
    Bidirectional
}

[bentoi]

I find it's quite unexpected to use DefaultStreamErrorCode to abort the read/write side of the stream when WriteAsync or ReadAsync is canceled.

I thought the conclusion of the discussion on #55485 was to do nothing on cancellation. With the assumption that the application will typically catch the exception and take action to either abort the write/read side of the stream (or possibly even dispose it).

Aborting the stream internally with DefaultStreamErrorCode implies that the application can no longer decide what error code to use other than DefaultStreamErrorCode.

[CarnaViire]

That's actually a very good point @bentoi. The way to supply your own code is to call Abort explicitly. But if you register it on the same cancellation token you pass to e.g. WriteAsync, I don't think we give any guarantee on the order in which cancellation callbacks are called (from reverse engineering CT code, they will be called in stack-like order) -- so that will not work.

The kind of super ugly workaround I can think of, is to have a second CT, that would first call abort with your code and then trigger the first CTS (which token is passed to the method) 🙈

[ManickaP]

The idea behind #55485 was to either have soft cancellation - you can still use the stream afterwards; or abort on cancellation - what we do now.
If you want to abort the stream with another code, you can abort it on your own. You wouldn't pass in your cancellation token to read/write but rather register for it on your own and call Abort(Read/Write) which will unblock the pending operation.

Neither solution is perfect, the question is which one is better for most of the users:

• aborting with default code, where some users like you will need to handle cancellation on their own
• allowing subsequent read/writes with potential to take unreasonable long since we don't have a way to cancel pending read/write in the underlying library I'm open to this, we can certainly do it from the technical point of view.

What we had in .NET until now, not aborting but also not allowing any subsequent operation, seems like the least expected behavior, so I'd rather not bring it back.

[bentoi]

To me, when dealing with a read or write operation on a socket, an IO stream or a Quic stream, the expectation is that if the operation raises an exception, there isn't much you can do after other than aborting/disposing the socket/stream. It's in particular true for writes where you can't figure out how much data was actually written before the exception is raised.

So a soft cancellation with the expectation that the application will abort the stream read/write side would be fine. I believe that's how other APIs such as Socket or IO stream also work.

Can't the QuicStream implementation disallow subsequent reads/writes without necessarily aborting the stream reads/writes (which ends up notifying the peer with this default stream error code)?

[ManickaP]

Can't the QuicStream implementation disallow subsequent reads/writes without necessarily aborting the stream reads/writes

That's what we did before and what we were not happy about. From the user perspective, you're not allowed to use the stream (particular side only) after cancellation, while from the protocol perspective, nothing is wrong with the stream. Moreover, the users would need to know that they should abort afterwards, otherwise we might actually close the writing side gracefully with dispose.

But we can discuss more the soft cancellation option and reconsider the current solution.
@bentoi how impactful this is for you? Are you using S.N.Quic? Understanding your scenario will help us make a more informed decision on this.

[bentoi]

I'm not using the Quic API yet but I would like to use it once it's released with .NET Core 7.0. I'm planning to use it for an application protocol similar to HTTP/3. For now, I'm using a TCP based transport similar to HTTP/2. Cancellation of a request aborts the underlying stream with a specific error code (like Http3ErrorCode.RequestCancelled). A request can also be canceled on connection closure and in this case I'd like to send another error code.

So basically, right now I'm using something like the following:

var linkedSource = CancellationTokenSource.CreateLinkedTokenSource(cancel, _closeCancellationSource.Token);
try
{
   stream.ReadAsync(linkedSource.Token);
}
catch (OperationCanceledException) when (_closeCancellationSource.IsCancellationRequested)
{
   stream.Abort(QuicAbortDirection.Read, ConnectionClosedErrorCode);
   throw new ConnectionClosedException();
}
catch (OperationCanceledException) when (cancel.IsCancellationRequested)
{
   stream.AbortRead(RequestCanceledErrorCode);
   throw;
}

I can re-arrange the code to instead do something like the following:

using _ = _closeCancellationSource.Token.Register(
     () => stream.Abort(QuicAbortDirection.Read, ConnectionClosedErrorCode));
try
{
     stream.ReadAsync(cancel);
}
catch (QuicException) when (_closeCancellationSource.IsCancellationRequested)
{
    // The stream was probably aborted on connection closure
    throw new ConnectionClosedException(); 
}
catch (OperationCanceledException)
{
    // request was canceled and stream aborted by the Quic implementation with default stream error code
    throw;
}

It's not more complicated so I'll use this instead.

However, I find that using the same default error code for different purposes isn't right. That's why I brought this up and questioned whether or not soft cancellation was a better approach. But I see your point about implicitly aborting the stream on cancellation.

[ManickaP]

@bentoi thanks for the examples! We'll certainly take your concerns and use cases into account and I'll re-open this discussion with my team.