Relax RandomAccess type requirements, make all Read*|Write* methods work with non-seekable files

[adamsitnik]

In .NET 6 we have introduced new type called RandomAccess that allows for reading and writing to specific file offset.

As of today, all it's Read* and Write* methods throw when given handle points to a non-seekable file like socket or pipe:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.cs

Line 251 in d033e1a

ThrowHelper.ThrowNotSupportedException_UnseekableStream();

But it's not a problem for it's internal implementation (used by FileStream):

runtime/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs

Lines 33 to 38 in d033e1a

	// The Windows implementation uses ReadFile, which ignores the offset if the handle
	// isn't seekable. We do the same manually with PRead vs Read, in order to enable
	// the function to be used by FileStream for all the same situations.
	int result = handle.CanSeek ?
	Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset) :
	Interop.Sys.Read(handle, bufPtr, buffer.Length);

And we use it's internal API surface to workaround this limitation is CoreLib (mind the call to RandomAccess.WriteAtOffset (internal), not RandomAccess.Write (public):

runtime/src/libraries/System.Private.CoreLib/src/System/IO/File.cs

Line 382 in d033e1a

RandomAccess.WriteAtOffset(sfh, bytes, 0);

We should relax the public requirements and make RandomAccess work with non-seekable files.

This is going to require:

• a breaking change doc
• implementation change (stop throwing exception)
• test changes (this test shows how to easily create a handle that points to non-seekable pipe)

Everything motioned above should be a single PR. In the same or separate PR, the Unix implementation of overloads that accept multiple buffers should start using readv and writev sys-calls. This should be relatively easy (just search for preadv and pwritev and reuse the patterns)

But it's going to allow to:

• switch from heavy FileStream to a very lightweight RandomAccess everywhere where we don't need buffering in BCL (and ofc outside of BCL). Sample perf gains: File.*AllText* optimizations #58167 (comment)
• remove a lot of code duplication. Example: https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.ValueTaskSource.cs

[ghost]

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

Issue Details

---

In .NET 6 we have introduced new type called RandomAccess that allows for reading and writing to specific file offset.

As of today, all it's Read* and Write* methods throw when given handle points to a non-seekable file like socket or pipe:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.cs

Line 251 in d033e1a

ThrowHelper.ThrowNotSupportedException_UnseekableStream();

But it's not a problem for it's internal implementation (used by FileStream):

runtime/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs

Lines 33 to 38 in d033e1a

	// The Windows implementation uses ReadFile, which ignores the offset if the handle
	// isn't seekable. We do the same manually with PRead vs Read, in order to enable
	// the function to be used by FileStream for all the same situations.
	int result = handle.CanSeek ?
	Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset) :
	Interop.Sys.Read(handle, bufPtr, buffer.Length);

And we use it's internal API surface to workaround this limitation is CoreLib (mind the call to RandomAccess.WriteAtOffset (internal), not RandomAccess.Write (public):

runtime/src/libraries/System.Private.CoreLib/src/System/IO/File.cs

Line 382 in d033e1a

RandomAccess.WriteAtOffset(sfh, bytes, 0);

We should relax the public requirements and make RandomAccess work with non-seekable files.

This is going to require:

• a breaking change doc
• implementation change (stop throwing exception)
• test changes (this test shows how to easily create a handle that points to non-seekable pipe)

Everything motioned above should be a single PR. In the same or separate PR, the Unix implementation of overloads that accept multiple buffers should start using readv and writev sys-calls. This should be relatively easy (just search for preadv and pwritev and reuse the patterns)

But it's going to allow to:

• switch from heavy FileStream to a very lightweight RandomAccess everywhere where we don't need buffering in BCL (and ofc outside of BCL). Sample perf gains: File.*AllText* optimizations #58167 (comment)
• remove a lot of code duplication. Example: https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.ValueTaskSource.cs

Author:	adamsitnik
Assignees:	-
Labels:	area-System.IO, up-for-grabs
Milestone:	7.0.0

[deeprobin]

@adamsitnik Whats the current state of this? Why is your PR closed?

[adamsitnik]

Whats the current state of this?

It's still on my TODO list for 7.0.

[alexrp]

@adamsitnik Is this likely to make it into .NET 7?

[jeffhandley]

Unfortunately, we won't be able to get to this in .NET 7.0. We will consider it again during our .NET 8 planning.

[alexrp]

What would it take to move this forward?

I've looked a fair amount at the RandomAccess/FileStream innards in the last few days, and this change honestly seems super straightforward. I could probably contribute a PR if the issue is allocating cycles to do the work.

Two potential problems I see are:

1. I think this might need API review sign-off? It wasn't explicitly written, but it seems API review signed off on these methods throwing for non-seekable handles. Can someone confirm if this would be necessary?
2. I'm not sure what bureaucracy/documentation around the breaking change aspect is needed (even if the break is fairly innocuous), and whether that's something that might be problematic if an external contributor does the work?

[adamsitnik]

It would most likely be a matter of resurrecting #58506, solving merge conflicts and writing a breaking change doc.

It would be great to take a fresh look at https://github.com/dotnet/runtime/pull/58506/files#r701031142 and see whether it can be improved.

[adamsitnik]

One of the main features of RandomAccess is thread-safety and ability to issue concurrent reads and writes. When we have offsets, it's easy: just do it for given offset. For non-seekable files, we would need to test the behavior and make sure that does what is expected and in the same way on Windows and Unix.

[adamsitnik]

I am going to send a PR for that.