How to implement Stream nowadays?

[drauch]

We currently need to implement our own Stream, using .NET 5, which wraps an asynchronous HTTP-based client API. Especially we need to:

• Write asynchronously
• Read asynchronously
• Perform asynchronous operations in Flush
• Perform asynchronous operations in Dispose

The client API does not provide any synchronous methods.
We noticed that the base class only forces us to override synchronous methods.

Our questions are:

• Should we throw in the synchronous methods or rather implement the sync-over-async anti pattern? (Read, Write, Flush)
• Which async Read/Write methods should we override? All of them? Only one specific?
• How to properly implement disposal? Is it enough to implement DisposeAsync? Do we need to implement Dispose and somehow call DisposeAsync in there using the sync-over-async anti pattern?

Best regards,
D.R.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[huoyaoyuan]

Related: #58216

Answers based on my knowledge:

Should we throw in the synchronous methods or rather implement the sync-over-async anti pattern? (Read, Write, Flush)

Anti-pattern should be better than not functional at all.

Which async Read/Write methods should we override? All of them? Only one specific?

Use the span/memory overload as core implementation, and delegates all others to them. For compatibility, the base implementations in Stream have to delegate to overloads accepting arrays.

How to properly implement disposal? Is it enough to implement DisposeAsync? Do we need to implement Dispose and somehow call DisposeAsync in there using the sync-over-async anti pattern?

The caller could call either Dispose or DisposeAsync. Ensure they both function correctly.

[ghost]

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

Issue Details

---

We currently need to implement our own Stream, using .NET 5, which wraps an asynchronous HTTP-based client API. Especially we need to:

• Write asynchronously
• Read asynchronously
• Perform asynchronous operations in Flush
• Perform asynchronous operations in Dispose

The client API does not provide any synchronous methods.
We noticed that the base class only forces us to override synchronous methods.

Our questions are:

• Should we throw in the synchronous methods or rather implement the sync-over-async anti pattern? (Read, Write, Flush)
• Which async Read/Write methods should we override? All of them? Only one specific?
• How to properly implement disposal? Is it enough to implement DisposeAsync? Do we need to implement Dispose and somehow call DisposeAsync in there using the sync-over-async anti pattern?

Best regards,
D.R.

Author:	drauch
Assignees:	-
Labels:	area-System.IO, untriaged
Milestone:	-

[jozkee]

You can take a look at the documentation for Stream, especially this part: https://docs.microsoft.com/en-us/dotnet/api/system.io.stream?view=net-5.0#notes-to-implementers.

Regarding your scenario, seems like you need an implementation that deals with HTTP: There may exist a better starting point to do that rather than implementing your own stream from scratch e.g: https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.networkstream?view=net-5.0.

cc @dotnet/ncl.

[fschmied]

You can take a look at the documentation for Stream, especially this part: https://docs.microsoft.com/en-us/dotnet/api/system.io.stream?view=net-5.0#notes-to-implementers.

@jozkee Are you sure that is up to date regarding async methods? Regarding those, it says:

When you implement a derived class of Stream, you must provide implementations for the Read(Byte[], Int32, Int32) and Write(Byte[], Int32, Int32) methods. The asynchronous methods ReadAsync(Byte[], Int32, Int32), WriteAsync(Byte[], Int32, Int32), and CopyToAsync(Stream) use the synchronous methods Read(Byte[], Int32, Int32) and Write(Byte[], Int32, Int32) in their implementations. Therefore, your implementations of Read(Byte[], Int32, Int32) and Write(Byte[], Int32, Int32) will work correctly with the asynchronous methods.

Following this advice, if I only have an async API underlying my stream implementation, I'll end up with:

• sync methods that do the sync-over-async antipattern,
• async methods that do the async-over-sync (over async) antipattern (in the Stream base class).

That can't be good, or can it?

[wfurt]

Well, it you map the perf will never be great @fschmied. The real question is what you are really trying to get. The more you implement and do without mapping the better IMHO. Seems like the link is minimalistic.
For many IO streams - like File - the sync/async really does not matter that much IMHO.

Also note that the 5.0 HttpClient has synchronous functions as well: https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.send?view=net-5.0

[fschmied]

Well, it you map the perf will never be great @fschmied. The real question is what you are really trying to get. The more you implement and do without mapping the better IMHO. Seems like the link is minimalistic.
For many IO streams - like File - the sync/async really does not matter that much IMHO.

@wfurt I didn't really mean performance. AFAIK sync-over-async is an anti-pattern that can cause grave problems. That's why I think the docs section mentioned above isn't really good advice, especially for the scenario described here.

Also note that the 5.0 HttpClient has synchronous functions as well

That's really cool and will make some thinks easier. In this specific case, it probably won't help; from the OP:

The client API does not provide any synchronous methods.

[fschmied]

@drauch I've just found that @stephentoub wrote a really nice article about subclassing Stream wrt sync and async in 2012: https://devblogs.microsoft.com/pfxteam/overriding-stream-asynchrony/ . I guess, it's still valid (although we didn't have Memory-based APIs back then):

Regarding the abstract, sync members:

The Read, Write, and Flush methods are the core synchronous mechanisms from reading and writing from and to a stream [...]
If your stream is only readable or only writable, or if you wanted to prohibit synchronous usage, you could choose to throw a NotSupportedException from the methods you don’t want consumers to be using, but you still must override the methods.

Regarding the async ones:

If you don’t override ReadAsync or WriteAsync, their default implementation in the Stream class will simply use the BeginRead/EndRead or BeginWrite/EndWrite methods to perform the work, returning a Task or Task to represent those calls, in a manner extremely similar to how Task.Factory.FromAsync creates Tasks from Begin/End pairs. So, if you only override the synchronous methods and none of the asynchronous ones, ReadAsync/WriteAsync will return a Task that represents Read/Write running on the ThreadPool, via the default implementation of BeginRead/EndRead/BeginWrite/EndWrite. [...] You can of course override ReadAsync/WriteAsync as well, regardless of whether you’ve overridden the Begin/End methods; this is in particular a good idea if you are able to support cancellation throughout the processing of the asynchronous operation, since the XxAsync methods accept a CancellationToken.
[...]
In some cases, you may have found it too difficult or error prone to implement the Begin/End methods, even if an asynchronous implementation would have been possible. In such cases, you might consider overriding ReadAsync/WriteAsync without overriding the Begin/End methods. This will typically be made significantly easier via the C# and Visual Basic support for writing async methods using await. Then, once you have your Task-based implementations, you might even consider overriding the Begin/End methods and implementing them in terms of the Task-based methods.

(The link to implement the Begin/End methods via Tasks in the original blog is broken, here's a working one: https://devblogs.microsoft.com/pfxteam/Using-Tasks-to-implement-the-APM-Pattern/ .)

[drauch]

Thank you @fschmied for digging into this.

So, in short, I should probably:

• Throw NotSupportedException in sync methods
• Override the ReadAsync/WriteAsync async Memory-based methods as suggested by @huoyaoyuan

Unfortunately, this still leaves me with my question of whether and how to properly implement IDisposable/IAsyncDisposable if I need to perform an async operation in the disposal of the stream. I guess I will implement IAsyncDisposable and sync-over-async in IDisposable.

Best regards,
D.R.