Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for non-seekable files to RandomAccess #58506

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.IO.Pipes;
using System.Threading;
using Microsoft.Win32.SafeHandles;
using Xunit;
Expand Down Expand Up @@ -48,17 +47,6 @@ public void ThrowsObjectDisposedExceptionForDisposedHandle()
Assert.Throws<ObjectDisposedException>(() => MethodUnderTest(handle, Array.Empty<byte>(), 0));
}

[Fact]
[SkipOnPlatform(TestPlatforms.Browser, "System.IO.Pipes aren't supported on browser")]
public void ThrowsNotSupportedExceptionForUnseekableFile()
{
using (var server = new AnonymousPipeServerStream(PipeDirection.Out))
using (SafeFileHandle handle = new SafeFileHandle(server.SafePipeHandle.DangerousGetHandle(), ownsHandle: false))
{
Assert.Throws<NotSupportedException>(() => MethodUnderTest(handle, Array.Empty<byte>(), 0));
}
}

[Theory]
[MemberData(nameof(GetSyncAsyncOptions))]
public void ThrowsArgumentOutOfRangeExceptionForNegativeFileOffset(FileOptions options)
Expand All @@ -72,7 +60,7 @@ public void ThrowsArgumentOutOfRangeExceptionForNegativeFileOffset(FileOptions o
}
}

protected static CancellationTokenSource GetCancelledTokenSource()
internal static CancellationTokenSource GetCancelledTokenSource()
{
CancellationTokenSource source = new CancellationTokenSource();
source.Cancel();
Expand Down
396 changes: 396 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/RandomAccess/NonSeekable.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO.Pipes;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Win32.SafeHandles;
using Xunit;

namespace System.IO.Tests
{
public class RandomAccess_NonSeekable_AsyncHandles : RandomAccess_NonSeekable
{
protected override PipeOptions PipeOptions => PipeOptions.Asynchronous;

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] // cancellable file IO is supported only on Windows
[InlineData(FileAccess.Read)]
[InlineData(FileAccess.Write)]
public async Task CancellationIsSupported(FileAccess access)
{
string pipeName = FileSystemTest.GetNamedPipeServerStreamName();
string pipePath = Path.GetFullPath($@"\\.\pipe\{pipeName}");

using (var server = new NamedPipeServerStream(pipeName, PipeDirection.InOut))
using (SafeFileHandle clientHandle = File.OpenHandle(pipePath, FileMode.Open, access, FileShare.None, FileOptions.Asynchronous))
{
await server.WaitForConnectionAsync();

Assert.True(clientHandle.IsAsync);

CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(250));
CancellationToken token = cts.Token;
byte[] buffer = new byte[1];

OperationCanceledException ex = await Assert.ThrowsAsync<OperationCanceledException>(
() => access == FileAccess.Write
? RandomAccess.WriteAsync(clientHandle, buffer, 0, token).AsTask()
: RandomAccess.ReadAsync(clientHandle, buffer, 0, token).AsTask());

Assert.Equal(token, ex.CancellationToken);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
<Compile Include="PathInternalTests.cs" />
<Compile Include="RandomAccess\Base.cs" />
<Compile Include="RandomAccess\GetLength.cs" />
<Compile Include="RandomAccess\NonSeekable.cs" />
<Compile Include="RandomAccess\NonSeekable_AsyncHandles.cs" />
<Compile Include="RandomAccess\Read.cs" />
<Compile Include="RandomAccess\ReadAsync.cs" />
<Compile Include="RandomAccess\ReadScatter.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,11 @@ internal static Exception GetIOError(int errorCode, string? path)
_bufferSize = memory.Length;
_memoryHandle = memory.Pin();
_overlapped = _fileHandle.ThreadPoolBinding!.AllocateNativeOverlapped(_preallocatedOverlapped);
_overlapped->OffsetLow = (int)fileOffset;
_overlapped->OffsetHigh = (int)(fileOffset >> 32);
if (_fileHandle.CanSeek)
{
_overlapped->OffsetLow = (int)fileOffset;
_overlapped->OffsetHigh = (int)(fileOffset >> 32);
}
return _overlapped;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span<byte> buffer

internal static unsafe long ReadScatterAtOffset(SafeFileHandle handle, IReadOnlyList<Memory<byte>> buffers, long fileOffset)
{
if (!handle.CanSeek)
{
return ReadScatterNoOffset(handle, buffers);
}

MemoryHandle[] handles = new MemoryHandle[buffers.Count];
Span<Interop.Sys.IOVector> vectors = buffers.Count <= IovStackThreshold ? stackalloc Interop.Sys.IOVector[IovStackThreshold] : new Interop.Sys.IOVector[buffers.Count];

Expand Down Expand Up @@ -74,6 +79,26 @@ internal static unsafe long ReadScatterAtOffset(SafeFileHandle handle, IReadOnly
return FileStreamHelpers.CheckFileCall(result, handle.Path);
}

private static long ReadScatterNoOffset(SafeFileHandle handle, IReadOnlyList<Memory<byte>> buffers)
{
long result = 0;

int buffersCount = buffers.Count;
for (int i = 0; i < buffersCount; i++)
{
Span<byte> buffer = buffers[i].Span;
int read = ReadAtOffset(handle, buffer, fileOffset: 0);
result += read;

if (read != buffer.Length)
{
return result; // stop on the first incomplete read
}
}

return result;
}

internal static ValueTask<int> ReadAtOffsetAsync(SafeFileHandle handle, Memory<byte> buffer, long fileOffset, CancellationToken cancellationToken, OSFileStreamStrategy? strategy = null)
=> ScheduleSyncReadAtOffsetAsync(handle, buffer, fileOffset, cancellationToken, strategy);

Expand All @@ -92,7 +117,7 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<by
// the function to be used by FileStream for all the same situations.
int bytesWritten = handle.CanSeek ?
Interop.Sys.PWrite(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length), fileOffset) :
Interop.Sys.Write(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length));
Interop.Sys.Write(handle, bufPtr, buffer.Length);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentoub I've removed the call to GetNumberOfBytesToWrite which as you know is used only for testing partial writes in DEBUG.

With pipes|sockets, every read operation is blocked until there is some data available. Even if it's a zero byte read to an empty buffer.
I wanted to test the scenario where we write X bytes to the pipe and try to read Y bytes from it, where Y > X and ensure that the read returns X bytes.
With partial writes emulation provided by GetNumberOfBytesToWrite, the read was returning X / 2 bytes (because this is how much we have written). I could perform more reads using exact length, but in reality we usually don't know how many bytes are available in the pipe|socket. I could not read until read returns 0 because when there is no data available, the read is blocked and does not return 0 ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's trading off testing one thing for another thing. It's not clear to me that's the right trade off, but if you think so, ok.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep in mind that we are still testing partial writes for seekable files. I just don't think that we can test non-seekable files using the same approach, as they just block the read when there is no data available.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just don't think that we can test non-seekable files using the same approach, as they just block the read when there is no data available.

If the write and read are issued concurrently, it should be fine.

I could perform more reads using exact length

That's how reading generally works, issuing repeated reads until all data expected is consumed.

I wanted to test the scenario where we write X bytes to the pipe and try to read Y bytes from it, where Y > X and ensure that the read returns X bytes.

What guarantees it'll return exactly X bytes at the OS level?


FileStreamHelpers.CheckFileCall(bytesWritten, handle.Path);
if (bytesWritten == buffer.Length)
Expand Down Expand Up @@ -130,6 +155,12 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly
return;
}

if (!handle.CanSeek)
{
WriteGatherNoOffset(handle, buffers);
return;
}

var handles = new MemoryHandle[buffersCount];
Span<Interop.Sys.IOVector> vectors = buffersCount <= IovStackThreshold ?
stackalloc Interop.Sys.IOVector[IovStackThreshold] :
Expand Down Expand Up @@ -202,6 +233,15 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly
}
}

private static void WriteGatherNoOffset(SafeFileHandle handle, IReadOnlyList<ReadOnlyMemory<byte>> buffers)
{
int buffersCount = buffers.Count;
for (int i = 0; i < buffersCount; i++)
{
WriteAtOffset(handle, buffers[i].Span, fileOffset: 0);
}
}

internal static ValueTask WriteAtOffsetAsync(SafeFileHandle handle, ReadOnlyMemory<byte> buffer, long fileOffset, CancellationToken cancellationToken, OSFileStreamStrategy? strategy = null)
=> ScheduleSyncWriteAtOffsetAsync(handle, buffer, fileOffset, cancellationToken, strategy);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span<b

try
{
overlapped = GetNativeOverlappedForAsyncHandle(handle.ThreadPoolBinding!, fileOffset, resetEvent);
overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, resetEvent);

fixed (byte* pinned = &MemoryMarshal.GetReference(buffer))
{
Expand Down Expand Up @@ -171,7 +171,7 @@ private static unsafe void WriteSyncUsingAsyncHandle(SafeFileHandle handle, Read

try
{
overlapped = GetNativeOverlappedForAsyncHandle(handle.ThreadPoolBinding!, fileOffset, resetEvent);
overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, resetEvent);

fixed (byte* pinned = &MemoryMarshal.GetReference(buffer))
{
Expand Down Expand Up @@ -681,15 +681,18 @@ private static async ValueTask WriteGatherAtOffsetMultipleSyscallsAsync(SafeFile
}
}

private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(ThreadPoolBoundHandle threadPoolBinding, long fileOffset, CallbackResetEvent resetEvent)
private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(SafeFileHandle handle, long fileOffset, CallbackResetEvent resetEvent)
{
// After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding
// to allocate a native overlapped and provide a valid callback.
NativeOverlapped* result = threadPoolBinding.AllocateNativeOverlapped(s_callback, resetEvent, null);
NativeOverlapped* result = handle.ThreadPoolBinding!.AllocateNativeOverlapped(s_callback, resetEvent, null);

// For pipes the offsets are ignored by the OS
result->OffsetLow = unchecked((int)fileOffset);
result->OffsetHigh = (int)(fileOffset >> 32);
// For pipes the offsets are in theory ignored by the OS
if (handle.CanSeek)
{
result->OffsetLow = unchecked((int)fileOffset);
result->OffsetHigh = (int)(fileOffset >> 32);
}

// From https://docs.microsoft.com/en-us/windows/win32/api/ioapiset/nf-ioapiset-getoverlappedresult:
// "If the hEvent member of the OVERLAPPED structure is NULL, the system uses the state of the hFile handle to signal when the operation has been completed.
Expand Down
Loading