From 880abcfbd2ebd4699717a9379fae3028f19df4c3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 16 Jun 2021 08:07:23 +0200 Subject: [PATCH 01/15] all RandomAccess methods should work for both sync and async file handles --- .../tests/RandomAccess/Base.cs | 47 ++--- .../tests/RandomAccess/GetLength.cs | 14 +- .../tests/RandomAccess/Mixed.cs | 136 ++++++++++++++ .../tests/RandomAccess/NoBuffering.Windows.cs | 73 +++++--- .../tests/RandomAccess/Read.cs | 39 ++-- .../tests/RandomAccess/ReadAsync.cs | 31 ++-- .../tests/RandomAccess/ReadScatter.cs | 38 ++-- .../tests/RandomAccess/ReadScatterAsync.cs | 46 ++--- .../tests/RandomAccess/Write.cs | 39 ++-- .../tests/RandomAccess/WriteAsync.cs | 31 ++-- .../tests/RandomAccess/WriteGather.cs | 38 ++-- .../tests/RandomAccess/WriteGatherAsync.cs | 46 ++--- .../tests/System.IO.FileSystem.Tests.csproj | 1 + .../SafeHandles/SafeFileHandle.Windows.cs | 64 ++++--- .../src/Resources/Strings.resx | 3 + .../System.Private.CoreLib.Shared.projitems | 3 + .../src/System/IO/RandomAccess.Unix.cs | 38 +--- .../src/System/IO/RandomAccess.Windows.cs | 175 ++++++++++++++---- .../src/System/IO/RandomAccess.cs | 74 +++++--- .../Net5CompatFileStreamStrategy.Windows.cs | 2 +- .../Strategies/WindowsFileStreamStrategy.cs | 2 +- 21 files changed, 621 insertions(+), 319 deletions(-) create mode 100644 src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.cs diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs index 8d876f30174cd0..5d8f1e879974d0 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // 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; @@ -12,12 +13,14 @@ public abstract class RandomAccess_Base : FileSystemTest { protected abstract T MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset); - protected virtual bool ShouldThrowForSyncHandle => false; - - protected virtual bool ShouldThrowForAsyncHandle => false; - protected virtual bool UsesOffsets => true; + public static IEnumerable GetSyncAsyncOptions() + { + yield return new object[] { FileOptions.None }; + yield return new object[] { FileOptions.Asynchronous }; + } + [Fact] public void ThrowsArgumentNullExceptionForNullHandle() { @@ -52,12 +55,12 @@ public void ThrowsNotSupportedExceptionForUnseekableFile() } } - [Fact] - public void ThrowsArgumentOutOfRangeExceptionForNegativeFileOffset() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsArgumentOutOfRangeExceptionForNegativeFileOffset(FileOptions options) { if (UsesOffsets) { - FileOptions options = ShouldThrowForAsyncHandle ? FileOptions.None : FileOptions.Asynchronous; using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, options: options)) { AssertExtensions.Throws("fileOffset", () => MethodUnderTest(handle, Array.Empty(), -1)); @@ -65,32 +68,6 @@ public void ThrowsArgumentOutOfRangeExceptionForNegativeFileOffset() } } - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/34582", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - [SkipOnPlatform(TestPlatforms.Browser, "async file IO is not supported on browser")] - public void ThrowsArgumentExceptionForAsyncFileHandle() - { - if (ShouldThrowForAsyncHandle) - { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, options: FileOptions.Asynchronous)) - { - AssertExtensions.Throws("handle", () => MethodUnderTest(handle, new byte[100], 0)); - } - } - } - - [Fact] - public void ThrowsArgumentExceptionForSyncFileHandle() - { - if (ShouldThrowForSyncHandle) - { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, options: FileOptions.None)) - { - AssertExtensions.Throws("handle", () => MethodUnderTest(handle, new byte[100], 0)); - } - } - } - protected static CancellationTokenSource GetCancelledTokenSource() { CancellationTokenSource source = new CancellationTokenSource(); @@ -98,12 +75,10 @@ protected static CancellationTokenSource GetCancelledTokenSource() return source; } - protected SafeFileHandle GetHandleToExistingFile(FileAccess access) + protected SafeFileHandle GetHandleToExistingFile(FileAccess access, FileOptions options) { string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[1]); - - FileOptions options = ShouldThrowForAsyncHandle ? FileOptions.None : FileOptions.Asynchronous; return File.OpenHandle(filePath, FileMode.Open, access, FileShare.None, options); } } diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/GetLength.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/GetLength.cs index 46e2914aed1679..1c21748f0198e8 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/GetLength.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/GetLength.cs @@ -13,23 +13,25 @@ protected override long MethodUnderTest(SafeFileHandle handle, byte[] bytes, lon protected override bool UsesOffsets => false; - [Fact] - public void ReturnsZeroForEmptyFile() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ReturnsZeroForEmptyFile(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, options: options)) { Assert.Equal(0, RandomAccess.GetLength(handle)); } } - [Fact] - public void ReturnsExactSizeForNonEmptyFiles() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ReturnsExactSizeForNonEmptyFiles(FileOptions options) { const int fileSize = 123; string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[fileSize]); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { Assert.Equal(fileSize, RandomAccess.GetLength(handle)); } diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.cs new file mode 100644 index 00000000000000..bf1c5ee697ec15 --- /dev/null +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.cs @@ -0,0 +1,136 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Runtime.InteropServices; +using System.Threading.Tasks; +using Microsoft.Win32.SafeHandles; +using Xunit; + +namespace System.IO.Tests +{ + [ActiveIssue("https://github.com/dotnet/runtime/issues/34582", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + [SkipOnPlatform(TestPlatforms.Browser, "async file IO is not supported on browser")] + public class RandomAccess_Mixed : FileSystemTest + { + [DllImport(Interop.Libraries.Kernel32, EntryPoint = "CreateFileW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false, ExactSpelling = true)] + private static extern unsafe SafeFileHandle CreateFileW( + string lpFileName, + FileAccess dwDesiredAccess, + FileShare dwShareMode, + IntPtr lpSecurityAttributes, + FileMode dwCreationDisposition, + int dwFlagsAndAttributes, + IntPtr hTemplateFile); + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UsingSingleBuffer(bool async) + { + string filePath = GetTestFilePath(); + FileOptions options = async ? FileOptions.Asynchronous : FileOptions.None; + + // we want to test all combinations: starting with sync|async write, then sync|async read etc + foreach (bool syncWrite in new bool[] { true, false }) + { + foreach (bool syncRead in new bool[] { true, false }) + { + // File.OpenHandle initializes ThreadPoolBinding for async file handles on Windows + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Create, FileAccess.ReadWrite, options: options)) + { + await Validate(handle, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); + } + + if (OperatingSystem.IsWindows()) + { + // tests code path where ThreadPoolBinding is not initialized + using (SafeFileHandle tpBindingNotInitialized = CreateFileW(filePath, FileAccess.ReadWrite, FileShare.None, IntPtr.Zero, FileMode.Create, (int)options, IntPtr.Zero)) + { + await Validate(tpBindingNotInitialized, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); + } + } + } + } + + static async Task Validate(SafeFileHandle handle, FileOptions options, bool[] syncWrites, bool[] syncReads) + { + byte[] writeBuffer = new byte[1]; + byte[] readBuffer = new byte[2]; + long fileOffset = 0; + + foreach (bool syncWrite in syncWrites) + { + foreach (bool syncRead in syncReads) + { + writeBuffer[0] = (byte)fileOffset; + + Assert.Equal(writeBuffer.Length, syncWrite ? RandomAccess.Write(handle, writeBuffer, fileOffset) : await RandomAccess.WriteAsync(handle, writeBuffer, fileOffset)); + Assert.Equal(writeBuffer.Length, syncRead ? RandomAccess.Read(handle, readBuffer, fileOffset) : await RandomAccess.ReadAsync(handle, readBuffer, fileOffset)); + Assert.Equal(writeBuffer[0], readBuffer[0]); + + fileOffset += 1; + } + } + } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UsingMultipleBuffers(bool async) + { + string filePath = GetTestFilePath(); + FileOptions options = async ? FileOptions.Asynchronous : FileOptions.None; + + foreach (bool syncWrite in new bool[] { true, false }) + { + foreach (bool syncRead in new bool[] { true, false }) + { + // File.OpenHandle initializes ThreadPoolBinding for async file handles on Windows + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Create, FileAccess.ReadWrite, options: options)) + { + await Validate(handle, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); + } + + if (OperatingSystem.IsWindows()) + { + // tests code path where ThreadPoolBinding is not initialized + using (SafeFileHandle tpBindingNotInitialized = CreateFileW(filePath, FileAccess.ReadWrite, FileShare.None, IntPtr.Zero, FileMode.Create, (int)options, IntPtr.Zero)) + { + await Validate(tpBindingNotInitialized, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); + } + } + } + } + + static async Task Validate(SafeFileHandle handle, FileOptions options, bool[] syncWrites, bool[] syncReads) + { + byte[] writeBuffer_1 = new byte[1]; + byte[] writeBuffer_2 = new byte[1]; + byte[] readBuffer_1 = new byte[1]; + byte[] readBuffer_2 = new byte[1]; + long fileOffset = 0; + + IReadOnlyList> readBuffers = new Memory[] { readBuffer_1, readBuffer_2 }; + IReadOnlyList> writeBuffers = new ReadOnlyMemory[] { writeBuffer_1, writeBuffer_2 }; + + foreach (bool syncWrite in syncWrites) + { + foreach (bool syncRead in syncReads) + { + writeBuffer_1[0] = (byte)fileOffset; + writeBuffer_2[0] = (byte)(fileOffset+1); + + Assert.Equal(writeBuffer_1.Length + writeBuffer_2.Length, syncWrite ? RandomAccess.Write(handle, writeBuffers, fileOffset) : await RandomAccess.WriteAsync(handle, writeBuffers, fileOffset)); + Assert.Equal(writeBuffer_1.Length + writeBuffer_2.Length, syncRead ? RandomAccess.Read(handle, readBuffers, fileOffset) : await RandomAccess.ReadAsync(handle, readBuffers, fileOffset)); + Assert.Equal(writeBuffer_1[0], readBuffer_1[0]); + Assert.Equal(writeBuffer_2[0], readBuffer_2[0]); + + fileOffset += 2; + } + } + } + } + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs index ea9982600945ea..585335c4cdcb1e 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Security.Cryptography; using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; @@ -14,15 +15,18 @@ public class RandomAccess_NoBuffering : FileSystemTest { private const FileOptions NoBuffering = (FileOptions)0x20000000; - [Fact] - public async Task ReadAsyncUsingSingleBuffer() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReadUsingSingleBuffer(bool async) { const int fileSize = 1_000_000; // 1 MB string filePath = GetTestFilePath(); byte[] expected = RandomNumberGenerator.GetBytes(fileSize); File.WriteAllBytes(filePath, expected); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: FileOptions.Asynchronous | NoBuffering)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, + options: FileOptions.Asynchronous | NoBuffering)) // to use Scatter&Gather APIs on Windows the handle MUST be opened for async IO using (SectorAlignedMemory buffer = SectorAlignedMemory.Allocate(Environment.SystemPageSize)) { int current = 0; @@ -39,7 +43,9 @@ public async Task ReadAsyncUsingSingleBuffer() // It's possible to get 0 if we are lucky and file size is a multiple of physical sector size. do { - current = await RandomAccess.ReadAsync(handle, buffer.Memory, fileOffset: total); + current = async + ? await RandomAccess.ReadAsync(handle, buffer.Memory, fileOffset: total) + : RandomAccess.Read(handle, buffer.GetSpan(), fileOffset: total); Assert.True(expected.AsSpan(total, current).SequenceEqual(buffer.GetSpan().Slice(0, current))); @@ -51,8 +57,10 @@ public async Task ReadAsyncUsingSingleBuffer() } } - [Fact] - public async Task ReadAsyncUsingMultipleBuffers() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReadAsyncUsingMultipleBuffers(bool async) { const int fileSize = 1_000_000; // 1 MB string filePath = GetTestFilePath(); @@ -66,16 +74,17 @@ public async Task ReadAsyncUsingMultipleBuffers() long current = 0; long total = 0; + IReadOnlyList> buffers = new Memory[] + { + buffer_1.Memory, + buffer_2.Memory, + }; + do { - current = await RandomAccess.ReadAsync( - handle, - new Memory[] - { - buffer_1.Memory, - buffer_2.Memory, - }, - fileOffset: total); + current = async + ? await RandomAccess.ReadAsync(handle, buffers, fileOffset: total) + : RandomAccess.Read(handle, buffers, fileOffset: total); int takeFromFirst = Math.Min(buffer_1.Memory.Length, (int)current); Assert.True(expected.AsSpan((int)total, takeFromFirst).SequenceEqual(buffer_1.GetSpan().Slice(0, takeFromFirst))); @@ -89,8 +98,10 @@ public async Task ReadAsyncUsingMultipleBuffers() } } - [Fact] - public async Task WriteAsyncUsingSingleBuffer() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task WriteUsingSingleBuffer(bool async) { string filePath = GetTestFilePath(); int bufferSize = Environment.SystemPageSize; @@ -107,18 +118,19 @@ public async Task WriteAsyncUsingSingleBuffer() int take = Math.Min(content.Length - total, bufferSize); content.AsSpan(total, take).CopyTo(buffer.GetSpan()); - total += await RandomAccess.WriteAsync( - handle, - buffer.Memory, - fileOffset: total); + total += async + ? await RandomAccess.WriteAsync(handle, buffer.Memory, fileOffset: total) + : RandomAccess.Write(handle, buffer.GetSpan(), fileOffset: total); } } Assert.Equal(content, File.ReadAllBytes(filePath)); } - [Fact] - public async Task WriteAsyncUsingMultipleBuffers() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task WriteAsyncUsingMultipleBuffers(bool async) { string filePath = GetTestFilePath(); int bufferSize = Environment.SystemPageSize; @@ -131,19 +143,20 @@ public async Task WriteAsyncUsingMultipleBuffers() { long total = 0; + IReadOnlyList> buffers = new ReadOnlyMemory[] + { + buffer_1.Memory, + buffer_2.Memory, + }; + while (total != fileSize) { content.AsSpan((int)total, bufferSize).CopyTo(buffer_1.GetSpan()); content.AsSpan((int)total + bufferSize, bufferSize).CopyTo(buffer_2.GetSpan()); - total += await RandomAccess.WriteAsync( - handle, - new ReadOnlyMemory[] - { - buffer_1.Memory, - buffer_2.Memory, - }, - fileOffset: total); + total += async + ? await RandomAccess.WriteAsync(handle, buffers, fileOffset: total) + : RandomAccess.Write(handle, buffers, fileOffset: total); } } diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Read.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Read.cs index 81021d04aee161..9559e7cebc12f5 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Read.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Read.cs @@ -13,39 +13,54 @@ public class RandomAccess_Read : RandomAccess_Base protected override int MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.Read(handle, bytes, fileOffset); - protected override bool ShouldThrowForAsyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform sync IO using async handle - - [Fact] - public void ThrowsOnWriteAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsOnWriteAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write, options)) { Assert.Throws(() => RandomAccess.Read(handle, new byte[1], 0)); } } - [Fact] - public void ReadToAnEmptyBufferReturnsZero() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ReadToAnEmptyBufferReturnsZero(FileOptions options) { string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[1]); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { Assert.Equal(0, RandomAccess.Read(handle, Array.Empty(), fileOffset: 0)); } } - [Fact] - public void ReadsBytesFromGivenFileAtGivenOffset() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void CanUseStackAllocatedMemory(FileOptions options) + { + string filePath = GetTestFilePath(); + File.WriteAllBytes(filePath, new byte[1] { 3 }); + + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) + { + Span stackAllocated = stackalloc byte[2]; + Assert.Equal(1, RandomAccess.Read(handle, stackAllocated, fileOffset: 0)); + Assert.Equal(3, stackAllocated[0]); + } + } + + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ReadsBytesFromGivenFileAtGivenOffset(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] expected = RandomNumberGenerator.GetBytes(fileSize); File.WriteAllBytes(filePath, expected); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { byte[] actual = new byte[fileSize + 1]; int current = 0; diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadAsync.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadAsync.cs index c18da9ecd50595..f23cd8f92f6ac9 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadAsync.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadAsync.cs @@ -17,13 +17,11 @@ public class RandomAccess_ReadAsync : RandomAccess_Base> protected override ValueTask MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.ReadAsync(handle, bytes, fileOffset); - protected override bool ShouldThrowForSyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform async IO using sync handle - - [Fact] - public async Task TaskAlreadyCanceledAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task TaskAlreadyCanceledAsync(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: options)) { CancellationTokenSource cts = GetCancelledTokenSource(); CancellationToken token = cts.Token; @@ -35,36 +33,39 @@ public async Task TaskAlreadyCanceledAsync() } } - [Fact] - public async Task ThrowsOnWriteAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ThrowsOnWriteAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write, options)) { await Assert.ThrowsAsync(async () => await RandomAccess.ReadAsync(handle, new byte[1], 0)); } } - [Fact] - public async Task ReadToAnEmptyBufferReturnsZeroAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ReadToAnEmptyBufferReturnsZeroAsync(FileOptions options) { string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[1]); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { Assert.Equal(0, await RandomAccess.ReadAsync(handle, Array.Empty(), fileOffset: 0)); } } - [Fact] - public async Task ReadsBytesFromGivenFileAtGivenOffsetAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task HappyPath(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] expected = RandomNumberGenerator.GetBytes(fileSize); File.WriteAllBytes(filePath, expected); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { byte[] actual = new byte[fileSize + 1]; int current = 0; diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatter.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatter.cs index 68058b6242fc79..7f51ea6e8478e2 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatter.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatter.cs @@ -13,48 +13,49 @@ public class RandomAccess_ReadScatter : RandomAccess_Base protected override long MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.Read(handle, new Memory[] { bytes }, fileOffset); - protected override bool ShouldThrowForAsyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform sync IO using async handle - - [Fact] - public void ThrowsArgumentNullExceptionForNullBuffers() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsArgumentNullExceptionForNullBuffers(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read, options)) { AssertExtensions.Throws("buffers", () => RandomAccess.Read(handle, buffers: null, 0)); } } - [Fact] - public void ThrowsOnWriteAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsOnWriteAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write, options)) { Assert.Throws(() => RandomAccess.Read(handle, new Memory[] { new byte[1] }, 0)); } } - [Fact] - public void ReadToAnEmptyBufferReturnsZero() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ReadToAnEmptyBufferReturnsZero(FileOptions options) { string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[1]); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { Assert.Equal(0, RandomAccess.Read(handle, new Memory[] { Array.Empty() }, fileOffset: 0)); } } - [Fact] - public void ReadsBytesFromGivenFileAtGivenOffset() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ReadsBytesFromGivenFileAtGivenOffset(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] expected = RandomNumberGenerator.GetBytes(fileSize); File.WriteAllBytes(filePath, expected); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { byte[] actual = new byte[fileSize + 1]; long current = 0; @@ -86,13 +87,14 @@ public void ReadsBytesFromGivenFileAtGivenOffset() } } - [Fact] - public void ReadToTheSameBufferOverwritesContent() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ReadToTheSameBufferOverwritesContent(FileOptions options) { string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[3] { 1, 2, 3 }); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { byte[] buffer = new byte[1]; Assert.Equal(buffer.Length + buffer.Length, RandomAccess.Read(handle, Enumerable.Repeat(buffer.AsMemory(), 2).ToList(), fileOffset: 0)); diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatterAsync.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatterAsync.cs index 68d631a6dc390a..250d9c316777da 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatterAsync.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/ReadScatterAsync.cs @@ -17,22 +17,21 @@ public class RandomAccess_ReadScatterAsync : RandomAccess_Base> protected override ValueTask MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.ReadAsync(handle, new Memory[] { bytes }, fileOffset); - protected override bool ShouldThrowForSyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform async IO using sync handle - - [Fact] - public void ThrowsArgumentNullExceptionForNullBuffers() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsArgumentNullExceptionForNullBuffers(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, options: options)) { AssertExtensions.Throws("buffers", () => RandomAccess.ReadAsync(handle, buffers: null, 0)); } } - [Fact] - public async Task TaskAlreadyCanceledAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task TaskAlreadyCanceledAsync(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: options)) { CancellationTokenSource cts = GetCancelledTokenSource(); CancellationToken token = cts.Token; @@ -44,36 +43,39 @@ public async Task TaskAlreadyCanceledAsync() } } - [Fact] - public async Task ThrowsOnWriteAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ThrowsOnWriteAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Write, options)) { await Assert.ThrowsAsync(async () => await RandomAccess.ReadAsync(handle, new Memory[] { new byte[1] }, 0)); } } - [Fact] - public async Task ReadToAnEmptyBufferReturnsZeroAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ReadToAnEmptyBufferReturnsZeroAsync(FileOptions options) { string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[1]); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { Assert.Equal(0, await RandomAccess.ReadAsync(handle, new Memory[] { Array.Empty() }, fileOffset: 0)); } } - [Fact] - public async Task ReadsBytesFromGivenFileAtGivenOffsetAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ReadsBytesFromGivenFileAtGivenOffsetAsync(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] expected = RandomNumberGenerator.GetBytes(fileSize); File.WriteAllBytes(filePath, expected); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { byte[] actual = new byte[fileSize + 1]; long current = 0; @@ -90,6 +92,7 @@ public async Task ReadsBytesFromGivenFileAtGivenOffsetAsync() new Memory[] { buffer_1, + Array.Empty(), buffer_2 }, fileOffset: total); @@ -104,13 +107,14 @@ public async Task ReadsBytesFromGivenFileAtGivenOffsetAsync() } } - [Fact] - public async Task ReadToTheSameBufferOverwritesContent() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ReadToTheSameBufferOverwritesContent(FileOptions options) { string filePath = GetTestFilePath(); File.WriteAllBytes(filePath, new byte[3] { 1, 2, 3 }); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Open, options: options)) { byte[] buffer = new byte[1]; Assert.Equal(buffer.Length + buffer.Length, await RandomAccess.ReadAsync(handle, Enumerable.Repeat(buffer.AsMemory(), 2).ToList(), fileOffset: 0)); diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Write.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Write.cs index abcac008dc6dd2..3cac633f53f769 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Write.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Write.cs @@ -12,35 +12,50 @@ public class RandomAccess_Write : RandomAccess_Base protected override int MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.Write(handle, bytes, fileOffset); - protected override bool ShouldThrowForAsyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform sync IO using async handle - - [Fact] - public void ThrowsOnReadAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsOnReadAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read, options)) { Assert.Throws(() => RandomAccess.Write(handle, new byte[1], 0)); } } - [Fact] - public void WriteUsingEmptyBufferReturnsZero() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void WriteUsingEmptyBufferReturnsZero(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write, options: options)) { Assert.Equal(0, RandomAccess.Write(handle, Array.Empty(), fileOffset: 0)); } } - [Fact] - public void WritesBytesFromGivenBufferToGivenFileAtGivenOffset() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void CanUseStackAllocatedMemory(FileOptions options) + { + string filePath = GetTestFilePath(); + Span stackAllocated = stackalloc byte[2] { 1, 2 }; + + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Create, FileAccess.Write, options: options)) + { + Assert.Equal(stackAllocated.Length, RandomAccess.Write(handle, stackAllocated, fileOffset: 0)); + } + + Assert.Equal(stackAllocated.ToArray(), File.ReadAllBytes(filePath)); + } + + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void WritesBytesFromGivenBufferToGivenFileAtGivenOffset(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] content = RandomNumberGenerator.GetBytes(fileSize); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, options)) { int total = 0; int current = 0; diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteAsync.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteAsync.cs index 074f5baac59442..b5c2399f9642b2 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteAsync.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteAsync.cs @@ -16,13 +16,11 @@ public class RandomAccess_WriteAsync : RandomAccess_Base> protected override ValueTask MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.WriteAsync(handle, bytes, fileOffset); - protected override bool ShouldThrowForSyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform async IO using sync handle - - [Fact] - public async Task TaskAlreadyCanceledAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task TaskAlreadyCanceledAsync(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: options)) { CancellationTokenSource cts = GetCancelledTokenSource(); CancellationToken token = cts.Token; @@ -34,32 +32,35 @@ public async Task TaskAlreadyCanceledAsync() } } - [Fact] - public async Task ThrowsOnReadAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ThrowsOnReadAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read, options)) { await Assert.ThrowsAsync(async () => await RandomAccess.WriteAsync(handle, new byte[1], 0)); } } - [Fact] - public async Task WriteUsingEmptyBufferReturnsZeroAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task WriteUsingEmptyBufferReturnsZeroAsync(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write, options: options)) { Assert.Equal(0, await RandomAccess.WriteAsync(handle, Array.Empty(), fileOffset: 0)); } } - [Fact] - public async Task WritesBytesFromGivenBufferToGivenFileAtGivenOffsetAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task WritesBytesFromGivenBufferToGivenFileAtGivenOffsetAsync(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] content = RandomNumberGenerator.GetBytes(fileSize); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, options)) { int total = 0; int current = 0; diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGather.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGather.cs index a9483d6c0eae21..5af52f0c61a178 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGather.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGather.cs @@ -14,44 +14,45 @@ public class RandomAccess_WriteGather : RandomAccess_Base protected override long MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.Write(handle, new ReadOnlyMemory[] { bytes }, fileOffset); - protected override bool ShouldThrowForAsyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform sync IO using async handle - - [Fact] - public void ThrowsArgumentNullExceptionForNullBuffers() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsArgumentNullExceptionForNullBuffers(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, options: options)) { AssertExtensions.Throws("buffers", () => RandomAccess.Write(handle, buffers: null, 0)); } } - [Fact] - public void ThrowsOnReadAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsOnReadAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read, options)) { Assert.Throws(() => RandomAccess.Write(handle, new ReadOnlyMemory[] { new byte[1] }, 0)); } } - [Fact] - public void WriteUsingEmptyBufferReturnsZero() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void WriteUsingEmptyBufferReturnsZero(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write, options: options)) { Assert.Equal(0, RandomAccess.Write(handle, new ReadOnlyMemory[] { Array.Empty() }, fileOffset: 0)); } } - [Fact] - public void WritesBytesFromGivenBuffersToGivenFileAtGivenOffset() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void WritesBytesFromGivenBuffersToGivenFileAtGivenOffset(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] content = RandomNumberGenerator.GetBytes(fileSize); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, options)) { long total = 0; long current = 0; @@ -81,8 +82,9 @@ public void WritesBytesFromGivenBuffersToGivenFileAtGivenOffset() Assert.Equal(content, File.ReadAllBytes(filePath)); } - [Fact] - public void DuplicatedBufferDuplicatesContent() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void DuplicatedBufferDuplicatesContent(FileOptions options) { const byte value = 1; const int repeatCount = 2; @@ -90,7 +92,7 @@ public void DuplicatedBufferDuplicatesContent() ReadOnlyMemory buffer = new byte[1] { value }; List> buffers = Enumerable.Repeat(buffer, repeatCount).ToList(); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Create, FileAccess.Write)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Create, FileAccess.Write, options: options)) { Assert.Equal(repeatCount, RandomAccess.Write(handle, buffers, fileOffset: 0)); } diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGatherAsync.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGatherAsync.cs index f8369eb13c7823..d6bd235efb745d 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGatherAsync.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGatherAsync.cs @@ -18,22 +18,21 @@ public class RandomAccess_WriteGatherAsync : RandomAccess_Base> protected override ValueTask MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) => RandomAccess.WriteAsync(handle, new ReadOnlyMemory[] { bytes }, fileOffset); - protected override bool ShouldThrowForSyncHandle - => OperatingSystem.IsWindows(); // on Windows we can NOT perform async IO using sync handle - - [Fact] - public void ThrowsArgumentNullExceptionForNullBuffers() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public void ThrowsArgumentNullExceptionForNullBuffers(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.Write, FileShare.None, options)) { AssertExtensions.Throws("buffers", () => RandomAccess.WriteAsync(handle, buffers: null, 0)); } } - [Fact] - public async Task TaskAlreadyCanceledAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task TaskAlreadyCanceledAsync(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.CreateNew, FileAccess.ReadWrite, options: options)) { CancellationTokenSource cts = GetCancelledTokenSource(); CancellationToken token = cts.Token; @@ -45,32 +44,35 @@ public async Task TaskAlreadyCanceledAsync() } } - [Fact] - public async Task ThrowsOnReadAccess() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task ThrowsOnReadAccess(FileOptions options) { - using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read)) + using (SafeFileHandle handle = GetHandleToExistingFile(FileAccess.Read, options)) { await Assert.ThrowsAsync(async () => await RandomAccess.WriteAsync(handle, new ReadOnlyMemory[] { new byte[1] }, 0)); } } - [Fact] - public async Task WriteUsingEmptyBufferReturnsZeroAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task WriteUsingEmptyBufferReturnsZeroAsync(FileOptions options) { - using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.Write, options: options)) { Assert.Equal(0, await RandomAccess.WriteAsync(handle, new ReadOnlyMemory[] { Array.Empty() }, fileOffset: 0)); } } - [Fact] - public async Task WritesBytesFromGivenBufferToGivenFileAtGivenOffsetAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task WritesBytesFromGivenBufferToGivenFileAtGivenOffsetAsync(FileOptions options) { const int fileSize = 4_001; string filePath = GetTestFilePath(); byte[] content = RandomNumberGenerator.GetBytes(fileSize); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, options)) { long total = 0; long current = 0; @@ -86,6 +88,7 @@ public async Task WritesBytesFromGivenBufferToGivenFileAtGivenOffsetAsync() new ReadOnlyMemory[] { buffer_1, + Array.Empty(), buffer_2 }, fileOffset: total); @@ -99,8 +102,9 @@ public async Task WritesBytesFromGivenBufferToGivenFileAtGivenOffsetAsync() Assert.Equal(content, File.ReadAllBytes(filePath)); } - [Fact] - public async Task DuplicatedBufferDuplicatesContentAsync() + [Theory] + [MemberData(nameof(GetSyncAsyncOptions))] + public async Task DuplicatedBufferDuplicatesContentAsync(FileOptions options) { const byte value = 1; const int repeatCount = 2; @@ -108,7 +112,7 @@ public async Task DuplicatedBufferDuplicatesContentAsync() ReadOnlyMemory buffer = new byte[1] { value }; List> buffers = Enumerable.Repeat(buffer, repeatCount).ToList(); - using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Create, FileAccess.Write, options: FileOptions.Asynchronous)) + using (SafeFileHandle handle = File.OpenHandle(filePath, FileMode.Create, FileAccess.Write, options: options)) { Assert.Equal(repeatCount, await RandomAccess.WriteAsync(handle, buffers, fileOffset: 0)); } diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index a271109ce2fc65..cbd93bc8d2de24 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -53,6 +53,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index 36710dcdbde5dd..90ee90bb66b1f0 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -48,7 +48,11 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA ownsHandle: true, options); - fileHandle.InitThreadPoolBindingIfNeeded(); + if ((options & FileOptions.Asynchronous) != 0) + { + // the handle has not been exposed yet, so we don't need to aquire a lock + fileHandle.InitThreadPoolBinding(); + } return fileHandle; } @@ -99,32 +103,50 @@ private static IntPtr NtCreateFile(string fullPath, FileMode mode, FileAccess ac } } - internal void InitThreadPoolBindingIfNeeded() + internal void EnsureThreadPoolBindingInitialized() { - if (IsAsync == true && ThreadPoolBinding == null) - { - // This is necessary for async IO using IO Completion ports via our - // managed Threadpool API's. This (theoretically) calls the OS's - // BindIoCompletionCallback method, and passes in a stub for the - // LPOVERLAPPED_COMPLETION_ROUTINE. This stub looks at the Overlapped - // struct for this request and gets a delegate to a managed callback - // from there, which it then calls on a threadpool thread. (We allocate - // our native OVERLAPPED structs 2 pointers too large and store EE state - // & GC handles there, one to an IAsyncResult, the other to a delegate.) - try - { - ThreadPoolBinding = ThreadPoolBoundHandle.BindHandle(this); - } - catch (ArgumentException ex) + if (IsAsync && ThreadPoolBinding == null) + { + Init(); + } + + void Init() // moved to a separate method so InitThreadPoolBindingIfNeeded can be inlined + { + lock (this) { - if (OwnsHandle) + if (ThreadPoolBinding == null) { - // We should close the handle so that the handle is not open until SafeFileHandle GC - Dispose(); + InitThreadPoolBinding(); } + } + } + } - throw new IOException(SR.IO_BindHandleFailed, ex); + private void InitThreadPoolBinding() + { + Debug.Assert(IsAsync); + + // This is necessary for async IO using IO Completion ports via our + // managed Threadpool API's. This (theoretically) calls the OS's + // BindIoCompletionCallback method, and passes in a stub for the + // LPOVERLAPPED_COMPLETION_ROUTINE. This stub looks at the Overlapped + // struct for this request and gets a delegate to a managed callback + // from there, which it then calls on a threadpool thread. (We allocate + // our native OVERLAPPED structs 2 pointers too large and store EE state + // & GC handles there, one to an IAsyncResult, the other to a delegate.) + try + { + ThreadPoolBinding = ThreadPoolBoundHandle.BindHandle(this); + } + catch (ArgumentException ex) + { + if (OwnsHandle) + { + // We should close the handle so that the handle is not open until SafeFileHandle GC + Dispose(); } + + throw new IOException(SR.IO_BindHandleFailed, ex); } } diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 99b032174387f5..1c557c01bd79d7 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -2662,6 +2662,9 @@ The file is too long. This operation is currently limited to supporting files less than 2 gigabytes in size. + + IO operation will not work. Most likely the file will become too long. + IO operation will not work. Most likely the file will become too long or the handle was not opened to support synchronous IO operations. diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index a189be9ac1517a..2290202eb10b80 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1505,6 +1505,9 @@ Common\Interop\Windows\Kernel32\Interop.GetModuleFileName.cs + + + Common\Interop\Windows\Kernel32\Interop.GetOverlappedResult.cs Common\Interop\Windows\Kernel32\Interop.GetProcessMemoryInfo.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs index 22a3934fe4089e..a47a601438cff2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs @@ -67,25 +67,12 @@ private static unsafe long ReadScatterAtOffset(SafeFileHandle handle, IReadOnlyL return FileStreamHelpers.CheckFileCall(result, path: null); } - private static ValueTask ReadAtOffsetAsync(SafeFileHandle handle, Memory buffer, long fileOffset, - CancellationToken cancellationToken) - { - return new ValueTask(Task.Factory.StartNew(static state => - { - var args = ((SafeFileHandle handle, Memory buffer, long fileOffset))state!; - return ReadAtOffset(args.handle, args.buffer.Span, args.fileOffset); - }, (handle, buffer, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); - } + private static ValueTask ReadAtOffsetAsync(SafeFileHandle handle, Memory buffer, long fileOffset, CancellationToken cancellationToken) + => ScheduleSyncReadAtOffsetAsync(handle, buffer, fileOffset, cancellationToken); private static ValueTask ReadScatterAtOffsetAsync(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset, CancellationToken cancellationToken) - { - return new ValueTask(Task.Factory.StartNew(static state => - { - var args = ((SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset))state!; - return ReadScatterAtOffset(args.handle, args.buffers, args.fileOffset); - }, (handle, buffers, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); - } + => ScheduleSyncReadScatterAtOffsetAsync(handle, buffers, fileOffset, cancellationToken); private static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset) { @@ -130,24 +117,11 @@ private static unsafe long WriteGatherAtOffset(SafeFileHandle handle, IReadOnlyL return FileStreamHelpers.CheckFileCall(result, path: null); } - private static ValueTask WriteAtOffsetAsync(SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset, - CancellationToken cancellationToken) - { - return new ValueTask(Task.Factory.StartNew(static state => - { - var args = ((SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset))state!; - return WriteAtOffset(args.handle, args.buffer.Span, args.fileOffset); - }, (handle, buffer, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); - } + private static ValueTask WriteAtOffsetAsync(SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset, CancellationToken cancellationToken) + => ScheduleSyncWriteAtOffsetAsync(handle, buffer, fileOffset, cancellationToken); private static ValueTask WriteGatherAtOffsetAsync(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset, CancellationToken cancellationToken) - { - return new ValueTask(Task.Factory.StartNew(static state => - { - var args = ((SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset))state!; - return WriteGatherAtOffset(args.handle, args.buffers, args.fileOffset); - }, (handle, buffers, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); - } + => ScheduleSyncWriteGatherAtOffsetAsync(handle, buffers, fileOffset, cancellationToken); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 5d198ccb0785c5..50ba43b8d7d6ae 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -14,6 +14,8 @@ namespace System.IO { public static partial class RandomAccess { + private static readonly IOCompletionCallback s_callback = AllocateCallback(); + internal static unsafe long GetFileLength(SafeFileHandle handle, string? path) { Interop.Kernel32.FILE_STANDARD_INFO info; @@ -28,57 +30,113 @@ internal static unsafe long GetFileLength(SafeFileHandle handle, string? path) internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer, long fileOffset, string? path = null) { - NativeOverlapped nativeOverlapped = GetNativeOverlapped(fileOffset); - int r = ReadFileNative(handle, buffer, syncUsingOverlapped: true, &nativeOverlapped, out int errorCode); + NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset); + ManualResetEvent? mres = null; + bool isSync = !handle.IsAsync; - if (r == -1) + if (!isSync) { - // For pipes, ERROR_BROKEN_PIPE is the normal end of the pipe. - if (errorCode == Interop.Errors.ERROR_BROKEN_PIPE) + mres = new ManualResetEvent(false); + + // 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. + // Use of file, named pipe, or communications-device handles for this purpose is discouraged. + // It is safer to use an event object because of the confusion that can occur when multiple simultaneous overlapped operations + // are performed on the same file, named pipe, or communications device. + // In this situation, there is no way to know which operation caused the object's state to be signaled." + // Since we want RandomAccess APIs to be thread-safe, we provide a dedicated wait handle. + nativeOverlapped->EventHandle = mres.SafeWaitHandle.DangerousGetHandle(); + } + + try + { + int result = ReadFileNative(handle, buffer, syncUsingOverlapped: isSync, nativeOverlapped, out int errorCode); + if (result != -1) { - r = 0; + return result; } - else + + if (errorCode == Interop.Errors.ERROR_IO_PENDING) { - if (errorCode == Interop.Errors.ERROR_INVALID_PARAMETER) + Debug.Assert(!isSync); + mres!.WaitOne(); + + if (Interop.Kernel32.GetOverlappedResult(handle, nativeOverlapped, ref result, bWait: false)) { - ThrowHelper.ThrowArgumentException_HandleNotSync(nameof(handle)); + return result; } - throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + } + + if (errorCode == Interop.Errors.ERROR_BROKEN_PIPE // For pipes, ERROR_BROKEN_PIPE is the normal end of the pipe. + || errorCode == Interop.Errors.ERROR_HANDLE_EOF) // logically success with 0 bytes read (read at end of file) + { + return 0; } - } - return r; + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + + } + finally + { + mres?.Dispose(); + } } internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset, string? path = null) { - NativeOverlapped nativeOverlapped = GetNativeOverlapped(fileOffset); - int r = WriteFileNative(handle, buffer, true, &nativeOverlapped, out int errorCode); + NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset); + ManualResetEvent? mres = null; + bool isSync = !handle.IsAsync; - if (r == -1) + if (!isSync) { - // For pipes, ERROR_NO_DATA is not an error, but the pipe is closing. - if (errorCode == Interop.Errors.ERROR_NO_DATA) + mres = new ManualResetEvent(false); + nativeOverlapped->EventHandle = mres.SafeWaitHandle.DangerousGetHandle(); + } + + try + { + int result = WriteFileNative(handle, buffer, syncUsingOverlapped: isSync, nativeOverlapped, out int errorCode); + if (result != -1) { - r = 0; + return result; } - else + + if (errorCode == Interop.Errors.ERROR_IO_PENDING) { - // ERROR_INVALID_PARAMETER may be returned for writes - // where the position is too large or for synchronous writes - // to a handle opened asynchronously. - if (errorCode == Interop.Errors.ERROR_INVALID_PARAMETER) + Debug.Assert(!isSync); + mres!.WaitOne(); + + if (Interop.Kernel32.GetOverlappedResult(handle, nativeOverlapped, ref result, bWait: false)) { - throw new IOException(SR.IO_FileTooLongOrHandleNotSync); + return result; } - throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); } - } - return r; + if (errorCode == Interop.Errors.ERROR_NO_DATA) // For pipes, ERROR_NO_DATA is not an error, but the pipe is closing. + { + return 0; + } + + // ERROR_INVALID_PARAMETER may be returned for writes + // where the position is too large or for synchronous writes + // to a handle opened asynchronously. + if (errorCode == Interop.Errors.ERROR_INVALID_PARAMETER) + { + throw new IOException(SR.IO_FileTooLong); + } + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + + } + finally + { + mres?.Dispose(); + } } internal static unsafe int ReadFileNative(SafeFileHandle handle, Span bytes, bool syncUsingOverlapped, NativeOverlapped* overlapped, out int errorCode) @@ -143,7 +201,9 @@ internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan ReadAtOffsetAsync(SafeFileHandle handle, Memory buffer, long fileOffset, CancellationToken cancellationToken) - => Map(QueueAsyncReadFile(handle, buffer, fileOffset, cancellationToken)); + => handle.IsAsync + ? Map(QueueAsyncReadFile(handle, buffer, fileOffset, cancellationToken)) + : ScheduleSyncReadAtOffsetAsync(handle, buffer, fileOffset, cancellationToken); private static ValueTask Map((SafeFileHandle.ValueTaskSource? vts, int errorCode) tuple) => tuple.vts != null @@ -153,6 +213,8 @@ private static ValueTask Map((SafeFileHandle.ValueTaskSource? vts, int erro internal static unsafe (SafeFileHandle.ValueTaskSource? vts, int errorCode) QueueAsyncReadFile( SafeFileHandle handle, Memory buffer, long fileOffset, CancellationToken cancellationToken) { + handle.EnsureThreadPoolBindingInitialized(); + SafeFileHandle.ValueTaskSource vts = handle.GetValueTaskSource(); try { @@ -200,11 +262,15 @@ internal static unsafe (SafeFileHandle.ValueTaskSource? vts, int errorCode) Queu } private static ValueTask WriteAtOffsetAsync(SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset, CancellationToken cancellationToken) - => Map(QueueAsyncWriteFile(handle, buffer, fileOffset, cancellationToken)); + => handle.IsAsync + ? Map(QueueAsyncWriteFile(handle, buffer, fileOffset, cancellationToken)) + : ScheduleSyncWriteAtOffsetAsync(handle, buffer, fileOffset, cancellationToken); internal static unsafe (SafeFileHandle.ValueTaskSource? vts, int errorCode) QueueAsyncWriteFile( SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset, CancellationToken cancellationToken) { + handle.EnsureThreadPoolBindingInitialized(); + SafeFileHandle.ValueTaskSource vts = handle.GetValueTaskSource(); try { @@ -291,6 +357,11 @@ private static long WriteGatherAtOffset(SafeFileHandle handle, IReadOnlyList ReadScatterAtOffsetAsync(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset, CancellationToken cancellationToken) { + if (!handle.IsAsync) + { + return ScheduleSyncReadScatterAtOffsetAsync(handle, buffers, fileOffset, cancellationToken); + } + if (CanUseScatterGatherWindowsAPIs(handle)) { long totalBytes = 0; @@ -358,6 +429,8 @@ private static async ValueTask ReadScatterAtOffsetSingleSyscallAsync(SafeF private static unsafe ValueTask ReadFileScatterAsync(SafeFileHandle handle, MemoryHandle pinnedSegments, int bytesToRead, long fileOffset, CancellationToken cancellationToken) { + handle.EnsureThreadPoolBindingInitialized(); + SafeFileHandle.ValueTaskSource vts = handle.GetValueTaskSource(); try { @@ -426,6 +499,11 @@ private static async ValueTask ReadScatterAtOffsetMultipleSyscallsAsync(Sa private static ValueTask WriteGatherAtOffsetAsync(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset, CancellationToken cancellationToken) { + if (!handle.IsAsync) + { + return ScheduleSyncWriteGatherAtOffsetAsync(handle, buffers, fileOffset, cancellationToken); + } + if (CanUseScatterGatherWindowsAPIs(handle)) { long totalBytes = 0; @@ -507,6 +585,8 @@ private static async ValueTask WriteGatherAtOffsetSingleSyscallAsync(SafeF private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, MemoryHandle pinnedSegments, int bytesToWrite, long fileOffset, CancellationToken cancellationToken) { + handle.EnsureThreadPoolBindingInitialized(); + SafeFileHandle.ValueTaskSource vts = handle.GetValueTaskSource(); try { @@ -545,14 +625,41 @@ private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, return new ValueTask(vts, vts.Version); } - private static NativeOverlapped GetNativeOverlapped(long fileOffset) + private static unsafe NativeOverlapped* GetNativeOverlappedForSynchronousOperation(SafeFileHandle handle, long fileOffset, NativeOverlapped stackAllocated = default) { - NativeOverlapped nativeOverlapped = default; + NativeOverlapped* result; + if (handle.IsAsync) + { + handle.EnsureThreadPoolBindingInitialized(); + + // After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding + // to allocate a native overlapped and provide a valid callback. + // Since we really don't care about the callback (because this is sync IO for async handle) + // and we are going to wait on WaitHandle anyway, we pass ThreadPoolBinding as a state. + // The callback is going to use it to free the native overlapped. + result = handle.ThreadPoolBinding!.AllocateNativeOverlapped(s_callback, handle.ThreadPoolBinding, null); + } + else + { + result = &stackAllocated; + } + // For pipes the offsets are ignored by the OS - nativeOverlapped.OffsetLow = unchecked((int)fileOffset); - nativeOverlapped.OffsetHigh = (int)(fileOffset >> 32); + result->OffsetLow = unchecked((int)fileOffset); + result->OffsetHigh = (int)(fileOffset >> 32); + + return result; + } - return nativeOverlapped; + private static unsafe IOCompletionCallback AllocateCallback() + { + return new IOCompletionCallback(Callback); + + static unsafe void Callback(uint errorCode, uint numBytes, NativeOverlapped* pOverlapped) + { + ThreadPoolBoundHandle threadPoolBoundHandle = (ThreadPoolBoundHandle)ThreadPoolBoundHandle.GetNativeOverlappedState(pOverlapped)!; + threadPoolBoundHandle.FreeNativeOverlapped(pOverlapped); + } } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.cs index 199d0d4ea91c51..6da5aa183b6616 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.cs @@ -37,14 +37,13 @@ public static long GetLength(SafeFileHandle handle) /// is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was opened for async IO. /// is negative. /// was not opened for reading. /// An I/O error occurred. /// Position of the file is not advanced. public static int Read(SafeFileHandle handle, Span buffer, long fileOffset) { - ValidateInput(handle, fileOffset, mustBeSync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); return ReadAtOffset(handle, buffer, fileOffset); } @@ -60,14 +59,13 @@ public static int Read(SafeFileHandle handle, Span buffer, long fileOffset /// is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was opened for async IO. /// is negative. /// was not opened for reading. /// An I/O error occurred. /// Position of the file is not advanced. public static long Read(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset) { - ValidateInput(handle, fileOffset, mustBeSync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); ValidateBuffers(buffers); return ReadScatterAtOffset(handle, buffers, fileOffset); @@ -85,14 +83,13 @@ public static long Read(SafeFileHandle handle, IReadOnlyList> buffe /// is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was not opened for async IO. /// is negative. /// was not opened for reading. /// An I/O error occurred. /// Position of the file is not advanced. public static ValueTask ReadAsync(SafeFileHandle handle, Memory buffer, long fileOffset, CancellationToken cancellationToken = default) { - ValidateInput(handle, fileOffset, mustBeAsync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); if (cancellationToken.IsCancellationRequested) { @@ -114,14 +111,13 @@ public static ValueTask ReadAsync(SafeFileHandle handle, Memory buffe /// is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was not opened for async IO. /// is negative. /// was not opened for reading. /// An I/O error occurred. /// Position of the file is not advanced. public static ValueTask ReadAsync(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset, CancellationToken cancellationToken = default) { - ValidateInput(handle, fileOffset, mustBeAsync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); ValidateBuffers(buffers); if (cancellationToken.IsCancellationRequested) @@ -143,14 +139,13 @@ public static ValueTask ReadAsync(SafeFileHandle handle, IReadOnlyList is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was opened for async IO. /// is negative. /// was not opened for writing. /// An I/O error occurred. /// Position of the file is not advanced. public static int Write(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset) { - ValidateInput(handle, fileOffset, mustBeSync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); return WriteAtOffset(handle, buffer, fileOffset); } @@ -166,14 +161,13 @@ public static int Write(SafeFileHandle handle, ReadOnlySpan buffer, long f /// is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was opened for async IO. /// is negative. /// was not opened for writing. /// An I/O error occurred. /// Position of the file is not advanced. public static long Write(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset) { - ValidateInput(handle, fileOffset, mustBeSync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); ValidateBuffers(buffers); return WriteGatherAtOffset(handle, buffers, fileOffset); @@ -191,14 +185,13 @@ public static long Write(SafeFileHandle handle, IReadOnlyList is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was not opened for async IO. /// is negative. /// was not opened for writing. /// An I/O error occurred. /// Position of the file is not advanced. public static ValueTask WriteAsync(SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset, CancellationToken cancellationToken = default) { - ValidateInput(handle, fileOffset, mustBeAsync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); if (cancellationToken.IsCancellationRequested) { @@ -220,14 +213,13 @@ public static ValueTask WriteAsync(SafeFileHandle handle, ReadOnlyMemory is invalid. /// The file is closed. /// The file does not support seeking (pipe or socket). - /// was not opened for async IO. /// is negative. /// was not opened for writing. /// An I/O error occurred. /// Position of the file is not advanced. public static ValueTask WriteAsync(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset, CancellationToken cancellationToken = default) { - ValidateInput(handle, fileOffset, mustBeAsync: OperatingSystem.IsWindows()); + ValidateInput(handle, fileOffset); ValidateBuffers(buffers); if (cancellationToken.IsCancellationRequested) @@ -238,7 +230,7 @@ public static ValueTask WriteAsync(SafeFileHandle handle, IReadOnlyList(IReadOnlyList buffers) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.buffers); } } + + private static ValueTask ScheduleSyncReadAtOffsetAsync(SafeFileHandle handle, Memory buffer, long fileOffset, CancellationToken cancellationToken) + { + return new ValueTask(Task.Factory.StartNew(static state => + { + var args = ((SafeFileHandle handle, Memory buffer, long fileOffset))state!; + return ReadAtOffset(args.handle, args.buffer.Span, args.fileOffset); + }, (handle, buffer, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); + } + + private static ValueTask ScheduleSyncReadScatterAtOffsetAsync(SafeFileHandle handle, IReadOnlyList> buffers, + long fileOffset, CancellationToken cancellationToken) + { + return new ValueTask(Task.Factory.StartNew(static state => + { + var args = ((SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset))state!; + return ReadScatterAtOffset(args.handle, args.buffers, args.fileOffset); + }, (handle, buffers, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); + } + + private static ValueTask ScheduleSyncWriteAtOffsetAsync(SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset, CancellationToken cancellationToken) + { + return new ValueTask(Task.Factory.StartNew(static state => + { + var args = ((SafeFileHandle handle, ReadOnlyMemory buffer, long fileOffset))state!; + return WriteAtOffset(args.handle, args.buffer.Span, args.fileOffset); + }, (handle, buffer, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); + } + + private static ValueTask ScheduleSyncWriteGatherAtOffsetAsync(SafeFileHandle handle, IReadOnlyList> buffers, + long fileOffset, CancellationToken cancellationToken) + { + return new ValueTask(Task.Factory.StartNew(static state => + { + var args = ((SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset))state!; + return WriteGatherAtOffset(args.handle, args.buffers, args.fileOffset); + }, (handle, buffers, fileOffset), cancellationToken, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default)); + } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs index ec74ccf1ac497d..309cec069a9d04 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs @@ -82,7 +82,7 @@ private void InitFromHandle(SafeFileHandle handle, FileAccess access, bool useAs private void InitFromHandleImpl(SafeFileHandle handle, bool useAsyncIO) { - handle.InitThreadPoolBindingIfNeeded(); + handle.EnsureThreadPoolBindingInitialized(); if (handle.CanSeek) SeekCore(handle, 0, SeekOrigin.Current); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs index dee1c5a954857b..6aee58038db5f6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs @@ -26,7 +26,7 @@ internal WindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access, Fil _share = share; _exposedHandle = true; - handle.InitThreadPoolBindingIfNeeded(); + handle.EnsureThreadPoolBindingInitialized(); if (handle.CanSeek) { From 805f105602f6b1aa240a72d43eca75c3b4c18032 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 16 Jun 2021 09:01:12 +0200 Subject: [PATCH 02/15] use the new RandomAccess APIs and get rid of "GetAwaiter().GetResult()" --- .../AsyncWindowsFileStreamStrategy.cs | 11 ---- .../SyncWindowsFileStreamStrategy.cs | 50 ------------------- .../Strategies/WindowsFileStreamStrategy.cs | 45 +++++++++++++++++ 3 files changed, 45 insertions(+), 61 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs index a194c4802d15c1..f077d94e36e140 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs @@ -22,14 +22,6 @@ internal AsyncWindowsFileStreamStrategy(string path, FileMode mode, FileAccess a internal override bool IsAsync => true; - public override int Read(byte[] buffer, int offset, int count) - { - ValueTask vt = ReadAsyncInternal(new Memory(buffer, offset, count), CancellationToken.None); - return vt.IsCompleted ? - vt.Result : - vt.AsTask().GetAwaiter().GetResult(); - } - public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => ReadAsyncInternal(new Memory(buffer, offset, count), cancellationToken).AsTask(); @@ -66,9 +58,6 @@ private unsafe ValueTask ReadAsyncInternal(Memory destination, Cancel : (errorCode == 0) ? ValueTask.FromResult(0) : ValueTask.FromException(HandleIOError(positionBefore, errorCode)); } - public override void Write(byte[] buffer, int offset, int count) - => WriteAsyncInternal(new ReadOnlyMemory(buffer, offset, count), CancellationToken.None).AsTask().GetAwaiter().GetResult(); - public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) => WriteAsyncInternal(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs index e3f111e2c71fa0..4091b2db65c82d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -22,10 +21,6 @@ internal SyncWindowsFileStreamStrategy(string path, FileMode mode, FileAccess ac internal override bool IsAsync => false; - public override int Read(byte[] buffer, int offset, int count) => ReadSpan(new Span(buffer, offset, count)); - - public override int Read(Span buffer) => ReadSpan(buffer); - public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { // If we weren't opened for asynchronous I/O, we still call to the base implementation so that @@ -46,19 +41,6 @@ public override ValueTask ReadAsync(Memory buffer, CancellationToken base.ReadAsync(buffer, cancellationToken); } - public override void Write(byte[] buffer, int offset, int count) - => WriteSpan(new ReadOnlySpan(buffer, offset, count)); - - public override void Write(ReadOnlySpan buffer) - { - if (_fileHandle.IsClosed) - { - ThrowHelper.ThrowObjectDisposedException_FileClosed(); - } - - WriteSpan(buffer); - } - public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { // If we weren't opened for asynchronous I/O, we still call to the base implementation so that @@ -80,37 +62,5 @@ public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationTo } public override Task FlushAsync(CancellationToken cancellationToken) => Task.CompletedTask; // no buffering = nothing to flush - - private unsafe int ReadSpan(Span destination) - { - if (!CanRead) - { - ThrowHelper.ThrowNotSupportedException_UnreadableStream(); - } - - Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed"); - - int r = RandomAccess.ReadAtOffset(_fileHandle, destination, _filePosition, _path); - Debug.Assert(r >= 0, $"RandomAccess.ReadAtOffset returned {r}."); - _filePosition += r; - - return r; - } - - private unsafe void WriteSpan(ReadOnlySpan source) - { - if (!CanWrite) - { - ThrowHelper.ThrowNotSupportedException_UnwritableStream(); - } - - Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed"); - - int r = RandomAccess.WriteAtOffset(_fileHandle, source, _filePosition, _path); - Debug.Assert(r >= 0, $"RandomAccess.WriteAtOffset returned {r}."); - _filePosition += r; - - UpdateLengthOnChangePosition(); - } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs index 6aee58038db5f6..bc4a7d0d98c9ed 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs @@ -265,5 +265,50 @@ protected unsafe void SetLengthCore(long value) _filePosition = value; } } + + public override int Read(byte[] buffer, int offset, int count) => ReadSpan(new Span(buffer, offset, count)); + + public override int Read(Span buffer) => ReadSpan(buffer); + + private unsafe int ReadSpan(Span destination) + { + if (_fileHandle.IsClosed) + { + ThrowHelper.ThrowObjectDisposedException_FileClosed(); + } + else if ((_access & FileAccess.Read) == 0) + { + ThrowHelper.ThrowNotSupportedException_UnreadableStream(); + } + + int r = RandomAccess.ReadAtOffset(_fileHandle, destination, _filePosition, _path); + Debug.Assert(r >= 0, $"RandomAccess.ReadAtOffset returned {r}."); + _filePosition += r; + + return r; + } + + public override void Write(byte[] buffer, int offset, int count) + => WriteSpan(new ReadOnlySpan(buffer, offset, count)); + + public override void Write(ReadOnlySpan buffer) => WriteSpan(buffer); + + private unsafe void WriteSpan(ReadOnlySpan source) + { + if (_fileHandle.IsClosed) + { + ThrowHelper.ThrowObjectDisposedException_FileClosed(); + } + else if ((_access & FileAccess.Write) == 0) + { + ThrowHelper.ThrowNotSupportedException_UnwritableStream(); + } + + int r = RandomAccess.WriteAtOffset(_fileHandle, source, _filePosition, _path); + Debug.Assert(r >= 0, $"RandomAccess.WriteAtOffset returned {r}."); + _filePosition += r; + + UpdateLengthOnChangePosition(); + } } } From 2e26b1dae6e42c77caa68d2e5f26bc00bf727be3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 16 Jun 2021 11:56:11 +0200 Subject: [PATCH 03/15] make the mixed sync and async IO tests Windows-specific --- .../{Mixed.cs => Mixed.Windows.cs} | 20 +++++++------------ .../tests/System.IO.FileSystem.Tests.csproj | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) rename src/libraries/System.IO.FileSystem/tests/RandomAccess/{Mixed.cs => Mixed.Windows.cs} (82%) diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.Windows.cs similarity index 82% rename from src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.cs rename to src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.Windows.cs index bf1c5ee697ec15..dcc7d47924f61f 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Mixed.Windows.cs @@ -10,7 +10,7 @@ namespace System.IO.Tests { [ActiveIssue("https://github.com/dotnet/runtime/issues/34582", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] - [SkipOnPlatform(TestPlatforms.Browser, "async file IO is not supported on browser")] + [PlatformSpecific(TestPlatforms.Windows)] public class RandomAccess_Mixed : FileSystemTest { [DllImport(Interop.Libraries.Kernel32, EntryPoint = "CreateFileW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false, ExactSpelling = true)] @@ -42,13 +42,10 @@ public async Task UsingSingleBuffer(bool async) await Validate(handle, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); } - if (OperatingSystem.IsWindows()) + // tests code path where ThreadPoolBinding is not initialized + using (SafeFileHandle tpBindingNotInitialized = CreateFileW(filePath, FileAccess.ReadWrite, FileShare.None, IntPtr.Zero, FileMode.Create, (int)options, IntPtr.Zero)) { - // tests code path where ThreadPoolBinding is not initialized - using (SafeFileHandle tpBindingNotInitialized = CreateFileW(filePath, FileAccess.ReadWrite, FileShare.None, IntPtr.Zero, FileMode.Create, (int)options, IntPtr.Zero)) - { - await Validate(tpBindingNotInitialized, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); - } + await Validate(tpBindingNotInitialized, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); } } } @@ -93,13 +90,10 @@ public async Task UsingMultipleBuffers(bool async) await Validate(handle, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); } - if (OperatingSystem.IsWindows()) + // tests code path where ThreadPoolBinding is not initialized + using (SafeFileHandle tpBindingNotInitialized = CreateFileW(filePath, FileAccess.ReadWrite, FileShare.None, IntPtr.Zero, FileMode.Create, (int)options, IntPtr.Zero)) { - // tests code path where ThreadPoolBinding is not initialized - using (SafeFileHandle tpBindingNotInitialized = CreateFileW(filePath, FileAccess.ReadWrite, FileShare.None, IntPtr.Zero, FileMode.Create, (int)options, IntPtr.Zero)) - { - await Validate(tpBindingNotInitialized, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); - } + await Validate(tpBindingNotInitialized, options, new bool[] { syncWrite, !syncWrite }, new bool[] { syncRead, !syncRead }); } } } diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index cbd93bc8d2de24..7d962d0f95198a 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -53,7 +53,6 @@ - @@ -72,6 +71,7 @@ + From ffb0f129a2871e3429b6ffd2062b9509bd3bda6d Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 16 Jun 2021 19:14:57 +0200 Subject: [PATCH 04/15] pass the local variable in explicit way --- .../src/System/IO/RandomAccess.Windows.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 50ba43b8d7d6ae..f18cac39725a7f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -30,7 +30,8 @@ internal static unsafe long GetFileLength(SafeFileHandle handle, string? path) internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer, long fileOffset, string? path = null) { - NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset); + NativeOverlapped stackAllocated = default; + NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset, &stackAllocated); ManualResetEvent? mres = null; bool isSync = !handle.IsAsync; @@ -86,7 +87,8 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset, string? path = null) { - NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset); + NativeOverlapped stackAllocated = default; + NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset, &stackAllocated); ManualResetEvent? mres = null; bool isSync = !handle.IsAsync; @@ -625,7 +627,7 @@ private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, return new ValueTask(vts, vts.Version); } - private static unsafe NativeOverlapped* GetNativeOverlappedForSynchronousOperation(SafeFileHandle handle, long fileOffset, NativeOverlapped stackAllocated = default) + private static unsafe NativeOverlapped* GetNativeOverlappedForSynchronousOperation(SafeFileHandle handle, long fileOffset, NativeOverlapped* stackAllocated) { NativeOverlapped* result; if (handle.IsAsync) @@ -641,7 +643,7 @@ private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, } else { - result = &stackAllocated; + result = stackAllocated; } // For pipes the offsets are ignored by the OS From 51f6d1c2921f26dea8f41364bb8d896a62edc4e2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 16 Jun 2021 19:15:58 +0200 Subject: [PATCH 05/15] Apply suggestions from code review Co-authored-by: campersau --- .../src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index 90ee90bb66b1f0..7d9cb101ca748b 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -110,7 +110,7 @@ internal void EnsureThreadPoolBindingInitialized() Init(); } - void Init() // moved to a separate method so InitThreadPoolBindingIfNeeded can be inlined + void Init() // moved to a separate method so EnsureThreadPoolBindingInitialized can be inlined { lock (this) { From 518fff85da75f36f5fb80f20903c96961613da30 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 29 Jun 2021 14:52:59 +0200 Subject: [PATCH 06/15] separate implementation for sync and async handles --- .../src/System/IO/RandomAccess.Windows.cs | 261 ++++++++---------- .../Strategies/FileStreamHelpers.Windows.cs | 27 +- .../Net5CompatFileStreamStrategy.Windows.cs | 24 +- 3 files changed, 169 insertions(+), 143 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index f18cac39725a7f..289e35749bba6b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -30,175 +30,143 @@ internal static unsafe long GetFileLength(SafeFileHandle handle, string? path) internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer, long fileOffset, string? path = null) { - NativeOverlapped stackAllocated = default; - NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset, &stackAllocated); - ManualResetEvent? mres = null; - bool isSync = !handle.IsAsync; - - if (!isSync) + if (handle.IsAsync) { - mres = new ManualResetEvent(false); - - // 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. - // Use of file, named pipe, or communications-device handles for this purpose is discouraged. - // It is safer to use an event object because of the confusion that can occur when multiple simultaneous overlapped operations - // are performed on the same file, named pipe, or communications device. - // In this situation, there is no way to know which operation caused the object's state to be signaled." - // Since we want RandomAccess APIs to be thread-safe, we provide a dedicated wait handle. - nativeOverlapped->EventHandle = mres.SafeWaitHandle.DangerousGetHandle(); + return ReadSyncUsingAsyncHandle(handle, buffer, fileOffset, path); } - try + NativeOverlapped overlapped = GetNativeOverlappedForSyncHandle(handle, fileOffset); + fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) { - int result = ReadFileNative(handle, buffer, syncUsingOverlapped: isSync, nativeOverlapped, out int errorCode); - if (result != -1) - { - return result; - } - - if (errorCode == Interop.Errors.ERROR_IO_PENDING) + if (Interop.Kernel32.ReadFile(handle, pinned, buffer.Length, out int numBytesRead, &overlapped) != 0) { - Debug.Assert(!isSync); - mres!.WaitOne(); - - if (Interop.Kernel32.GetOverlappedResult(handle, nativeOverlapped, ref result, bWait: false)) - { - return result; - } - - errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + return numBytesRead; } - if (errorCode == Interop.Errors.ERROR_BROKEN_PIPE // For pipes, ERROR_BROKEN_PIPE is the normal end of the pipe. - || errorCode == Interop.Errors.ERROR_HANDLE_EOF) // logically success with 0 bytes read (read at end of file) + int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + switch (errorCode) { - return 0; + case Interop.Errors.ERROR_HANDLE_EOF: + // https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile#synchronization-and-file-position : + // "If lpOverlapped is not NULL, then when a synchronous read operation reaches the end of a file, + // ReadFile returns FALSE and GetLastError returns ERROR_HANDLE_EOF" + return numBytesRead; + case Interop.Errors.ERROR_BROKEN_PIPE: + // For pipes, ERROR_BROKEN_PIPE is the normal end of the pipe. + return 0; + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); } - - throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); - - } - finally - { - mres?.Dispose(); } } - internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset, string? path = null) + private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span buffer, long fileOffset, string? path) { - NativeOverlapped stackAllocated = default; - NativeOverlapped* nativeOverlapped = GetNativeOverlappedForSynchronousOperation(handle, fileOffset, &stackAllocated); - ManualResetEvent? mres = null; - bool isSync = !handle.IsAsync; + using ManualResetEvent mres = new ManualResetEvent(false); + NativeOverlapped* overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, mres); - if (!isSync) - { - mres = new ManualResetEvent(false); - nativeOverlapped->EventHandle = mres.SafeWaitHandle.DangerousGetHandle(); - } - - try + fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) { - int result = WriteFileNative(handle, buffer, syncUsingOverlapped: isSync, nativeOverlapped, out int errorCode); - if (result != -1) - { - return result; - } + int readFileResult = Interop.Kernel32.ReadFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped); + Debug.Assert(readFileResult == 0, "ReadFile should always return 0 for async or failed operations"); + int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); if (errorCode == Interop.Errors.ERROR_IO_PENDING) { - Debug.Assert(!isSync); - mres!.WaitOne(); + mres.WaitOne(); - if (Interop.Kernel32.GetOverlappedResult(handle, nativeOverlapped, ref result, bWait: false)) + int result = 0; + if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false)) { + Debug.Assert(result >= 0 && result <= buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request"); return result; } errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); } - if (errorCode == Interop.Errors.ERROR_NO_DATA) // For pipes, ERROR_NO_DATA is not an error, but the pipe is closing. - { - return 0; - } - - // ERROR_INVALID_PARAMETER may be returned for writes - // where the position is too large or for synchronous writes - // to a handle opened asynchronously. - if (errorCode == Interop.Errors.ERROR_INVALID_PARAMETER) + switch (errorCode) { - throw new IOException(SR.IO_FileTooLong); + case Interop.Errors.ERROR_HANDLE_EOF: // logically success with 0 bytes read (read at end of file) + case Interop.Errors.ERROR_BROKEN_PIPE: + // EOF on a pipe. Callback will not be called. + // We clear the overlapped status bit for this special case (failure + // to do so looks like we are freeing a pending overlapped later). + overlapped->InternalLow = IntPtr.Zero; + return 0; + + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); } - - throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); - - } - finally - { - mres?.Dispose(); } } - internal static unsafe int ReadFileNative(SafeFileHandle handle, Span bytes, bool syncUsingOverlapped, NativeOverlapped* overlapped, out int errorCode) + internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset, string? path = null) { - Debug.Assert(handle != null, "handle != null"); - - int r; - int numBytesRead = 0; - - fixed (byte* p = &MemoryMarshal.GetReference(bytes)) + if (handle.IsAsync) { - r = overlapped == null || syncUsingOverlapped ? - Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, overlapped) : - Interop.Kernel32.ReadFile(handle, p, bytes.Length, IntPtr.Zero, overlapped); + return WriteSyncUsingAsyncHandle(handle, buffer, fileOffset, path); } - if (r == 0) + NativeOverlapped overlapped = GetNativeOverlappedForSyncHandle(handle, fileOffset); + fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) { - errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); - - if (syncUsingOverlapped && errorCode == Interop.Errors.ERROR_HANDLE_EOF) + if (Interop.Kernel32.WriteFile(handle, pinned, buffer.Length, out int numBytesWritten, &overlapped) != 0) { - // https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile#synchronization-and-file-position : - // "If lpOverlapped is not NULL, then when a synchronous read operation reaches the end of a file, - // ReadFile returns FALSE and GetLastError returns ERROR_HANDLE_EOF" - return numBytesRead; + return numBytesWritten; } - return -1; - } - else - { - errorCode = 0; - return numBytesRead; + int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + switch (errorCode) + { + case Interop.Errors.ERROR_NO_DATA: // EOF on a pipe + return 0; + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + } } } - internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan buffer, bool syncUsingOverlapped, NativeOverlapped* overlapped, out int errorCode) + private static unsafe int WriteSyncUsingAsyncHandle(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset, string? path) { - Debug.Assert(handle != null, "handle != null"); - - int numBytesWritten = 0; - int r; + using ManualResetEvent mres = new ManualResetEvent(false); + NativeOverlapped* overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, mres); - fixed (byte* p = &MemoryMarshal.GetReference(buffer)) + fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) { - r = overlapped == null || syncUsingOverlapped ? - Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, overlapped) : - Interop.Kernel32.WriteFile(handle, p, buffer.Length, IntPtr.Zero, overlapped); - } + int writeFileResult = Interop.Kernel32.WriteFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped); + Debug.Assert(writeFileResult == 0, "WriteFile should always return 0 for async or failed operations"); - if (r == 0) - { - errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); - return -1; - } - else - { - errorCode = 0; - return numBytesWritten; + int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + if (errorCode == Interop.Errors.ERROR_IO_PENDING) + { + mres.WaitOne(); + + int result = 0; + if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false)) + { + Debug.Assert(result >= 0 && result <= buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request"); + return result; + } + + errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + } + + switch (errorCode) + { + case Interop.Errors.ERROR_NO_DATA: + // For pipes, ERROR_NO_DATA is not an error, but the pipe is closing. + return 0; + + case Interop.Errors.ERROR_INVALID_PARAMETER: + // ERROR_INVALID_PARAMETER may be returned for writes + // where the position is too large or for synchronous writes + // to a handle opened asynchronously. + throw new IOException(SR.IO_FileTooLong); + + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + } } } @@ -627,29 +595,42 @@ private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, return new ValueTask(vts, vts.Version); } - private static unsafe NativeOverlapped* GetNativeOverlappedForSynchronousOperation(SafeFileHandle handle, long fileOffset, NativeOverlapped* stackAllocated) + private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(SafeFileHandle handle, long fileOffset, ManualResetEvent mres) { - NativeOverlapped* result; - if (handle.IsAsync) - { - handle.EnsureThreadPoolBindingInitialized(); - - // After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding - // to allocate a native overlapped and provide a valid callback. - // Since we really don't care about the callback (because this is sync IO for async handle) - // and we are going to wait on WaitHandle anyway, we pass ThreadPoolBinding as a state. - // The callback is going to use it to free the native overlapped. - result = handle.ThreadPoolBinding!.AllocateNativeOverlapped(s_callback, handle.ThreadPoolBinding, null); - } - else - { - result = stackAllocated; - } + Debug.Assert(handle.IsAsync); + + handle.EnsureThreadPoolBindingInitialized(); + + // After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding + // to allocate a native overlapped and provide a valid callback. + // Since we really don't care about the callback (because this is sync IO for async handle) + // and we are going to wait on WaitHandle anyway, we pass ThreadPoolBinding as a state. + // The callback is going to use it to free the native overlapped. + NativeOverlapped* result = handle.ThreadPoolBinding!.AllocateNativeOverlapped(s_callback, handle.ThreadPoolBinding, null); // For pipes the offsets are ignored by the OS 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. + // Use of file, named pipe, or communications-device handles for this purpose is discouraged. + // It is safer to use an event object because of the confusion that can occur when multiple simultaneous overlapped operations + // are performed on the same file, named pipe, or communications device. + // In this situation, there is no way to know which operation caused the object's state to be signaled." + // Since we want RandomAccess APIs to be thread-safe, we provide a dedicated wait handle. + result->EventHandle = mres.SafeWaitHandle.DangerousGetHandle(); + + return result; + } + + private static NativeOverlapped GetNativeOverlappedForSyncHandle(SafeFileHandle handle, long fileOffset) + { + Debug.Assert(!handle.IsAsync); + + NativeOverlapped result = default; + result.OffsetLow = unchecked((int)fileOffset); + result.OffsetHigh = (int)(fileOffset >> 32); return result; } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs index 39275721010fb1..087c51f6f2dc07 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs @@ -189,6 +189,31 @@ internal static unsafe void SetFileLength(SafeFileHandle handle, string? path, l } + internal static unsafe int ReadFileNative(SafeFileHandle handle, Span bytes, NativeOverlapped* overlapped, out int errorCode) + { + Debug.Assert(handle != null, "handle != null"); + + int r; + int numBytesRead = 0; + + fixed (byte* p = &MemoryMarshal.GetReference(bytes)) + { + r = overlapped == null + ? Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, overlapped) + : Interop.Kernel32.ReadFile(handle, p, bytes.Length, IntPtr.Zero, overlapped); + } + + if (r == 0) + { + errorCode = GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + return -1; + } + else + { + errorCode = 0; + return numBytesRead; + } + } internal static async Task AsyncModeCopyToAsync(SafeFileHandle handle, string? path, bool canSeek, long filePosition, Stream destination, int bufferSize, CancellationToken cancellationToken) { @@ -265,7 +290,7 @@ internal static async Task AsyncModeCopyToAsync(SafeFileHandle handle, string? p } // Kick off the read. - synchronousSuccess = RandomAccess.ReadFileNative(handle, copyBuffer, false, readAwaitable._nativeOverlapped, out errorCode) >= 0; + synchronousSuccess = ReadFileNative(handle, copyBuffer, readAwaitable._nativeOverlapped, out errorCode) >= 0; } // If the operation did not synchronously succeed, it either failed or initiated the asynchronous operation. diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs index 309cec069a9d04..addfebcadaccba 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/Net5CompatFileStreamStrategy.Windows.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; @@ -1005,14 +1006,33 @@ private unsafe int ReadFileNative(SafeFileHandle handle, Span bytes, Nativ { Debug.Assert((_useAsyncIO && overlapped != null) || (!_useAsyncIO && overlapped == null), "Async IO and overlapped parameters inconsistent in call to ReadFileNative."); - return RandomAccess.ReadFileNative(handle, bytes, false, overlapped, out errorCode); + return FileStreamHelpers.ReadFileNative(handle, bytes, overlapped, out errorCode); } private unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan buffer, NativeOverlapped* overlapped, out int errorCode) { Debug.Assert((_useAsyncIO && overlapped != null) || (!_useAsyncIO && overlapped == null), "Async IO and overlapped parameters inconsistent in call to WriteFileNative."); - return RandomAccess.WriteFileNative(handle, buffer, false, overlapped, out errorCode); + int numBytesWritten = 0; + int r; + + fixed (byte* p = &MemoryMarshal.GetReference(buffer)) + { + r = overlapped == null + ? Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, overlapped) + : Interop.Kernel32.WriteFile(handle, p, buffer.Length, IntPtr.Zero, overlapped); + } + + if (r == 0) + { + errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + return -1; + } + else + { + errorCode = 0; + return numBytesWritten; + } } public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) From 44a7440fd7a9f2d9bb3fd443969e44b0e54e29aa Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 30 Jun 2021 10:51:04 +0200 Subject: [PATCH 07/15] handle lucky path properly: when async operation is executed immediately and GetLastError() returns SUCCESS instead of PENDING --- .../src/System/IO/RandomAccess.Windows.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 289e35749bba6b..9bb429e96677ab 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -71,7 +71,7 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span Date: Wed, 30 Jun 2021 13:45:20 +0200 Subject: [PATCH 08/15] remove race condition --- .../src/System/IO/RandomAccess.Windows.cs | 179 ++++++++++++------ 1 file changed, 116 insertions(+), 63 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 9bb429e96677ab..302141908c74c8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -62,42 +62,58 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span buffer, long fileOffset, string? path) { - using ManualResetEvent mres = new ManualResetEvent(false); - NativeOverlapped* overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, mres); + handle.EnsureThreadPoolBindingInitialized(); - fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) + CallbackResetEvent resetEvent = new CallbackResetEvent(false, handle.ThreadPoolBinding!); + NativeOverlapped* overlapped = null; + + try { - int readFileResult = Interop.Kernel32.ReadFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped); - Debug.Assert(readFileResult == 0, "ReadFile should always return 0 for async or failed operations"); + overlapped = GetNativeOverlappedForAsyncHandle(handle.ThreadPoolBinding!, fileOffset, resetEvent); - int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); - if (errorCode == Interop.Errors.ERROR_IO_PENDING || errorCode == Interop.Errors.ERROR_SUCCESS) + fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) { - mres.WaitOne(); + int readFileResult = Interop.Kernel32.ReadFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped); + Debug.Assert(readFileResult == 0, "ReadFile should always return 0 for async or failed operations"); - int result = 0; - if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false)) + int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + if (errorCode == Interop.Errors.ERROR_IO_PENDING || errorCode == Interop.Errors.ERROR_SUCCESS) { - Debug.Assert(result >= 0 && result <= buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request"); - return result; + resetEvent.WaitOne(); + + int result = 0; + if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false)) + { + Debug.Assert(result >= 0 && result <= buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request"); + return result; + } + + errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); } - errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); - } + switch (errorCode) + { + case Interop.Errors.ERROR_HANDLE_EOF: // logically success with 0 bytes read (read at end of file) + case Interop.Errors.ERROR_BROKEN_PIPE: + // EOF on a pipe. Callback will not be called. + // We clear the overlapped status bit for this special case (failure + // to do so looks like we are freeing a pending overlapped later). + overlapped->InternalLow = IntPtr.Zero; + return 0; - switch (errorCode) + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + } + } + } + finally + { + if (overlapped != null) { - case Interop.Errors.ERROR_HANDLE_EOF: // logically success with 0 bytes read (read at end of file) - case Interop.Errors.ERROR_BROKEN_PIPE: - // EOF on a pipe. Callback will not be called. - // We clear the overlapped status bit for this special case (failure - // to do so looks like we are freeing a pending overlapped later). - overlapped->InternalLow = IntPtr.Zero; - return 0; - - default: - throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + resetEvent.FreeNativeOverlappedIfItIsSafe(overlapped); } + + resetEvent.Dispose(); } } @@ -129,44 +145,60 @@ internal static unsafe int WriteAtOffset(SafeFileHandle handle, ReadOnlySpan buffer, long fileOffset, string? path) { - using ManualResetEvent mres = new ManualResetEvent(false); - NativeOverlapped* overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, mres); + handle.EnsureThreadPoolBindingInitialized(); - fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) + CallbackResetEvent resetEvent = new CallbackResetEvent(false, handle.ThreadPoolBinding!); + NativeOverlapped* overlapped = null; + + try { - int writeFileResult = Interop.Kernel32.WriteFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped); - Debug.Assert(writeFileResult == 0, "WriteFile should always return 0 for async or failed operations"); + overlapped = GetNativeOverlappedForAsyncHandle(handle.ThreadPoolBinding!, fileOffset, resetEvent); - int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); - if (errorCode == Interop.Errors.ERROR_IO_PENDING || errorCode == Interop.Errors.ERROR_SUCCESS) + fixed (byte* pinned = &MemoryMarshal.GetReference(buffer)) { - mres.WaitOne(); + int writeFileResult = Interop.Kernel32.WriteFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped); + Debug.Assert(writeFileResult == 0, "WriteFile should always return 0 for async or failed operations"); - int result = 0; - if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false)) + int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + if (errorCode == Interop.Errors.ERROR_IO_PENDING || errorCode == Interop.Errors.ERROR_SUCCESS) { - Debug.Assert(result >= 0 && result <= buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request"); - return result; - } + resetEvent.WaitOne(); - errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); - } + int result = 0; + if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false)) + { + Debug.Assert(result >= 0 && result <= buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request"); + return result; + } - switch (errorCode) - { - case Interop.Errors.ERROR_NO_DATA: - // For pipes, ERROR_NO_DATA is not an error, but the pipe is closing. - return 0; + errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle); + } - case Interop.Errors.ERROR_INVALID_PARAMETER: - // ERROR_INVALID_PARAMETER may be returned for writes - // where the position is too large or for synchronous writes - // to a handle opened asynchronously. - throw new IOException(SR.IO_FileTooLong); + switch (errorCode) + { + case Interop.Errors.ERROR_NO_DATA: + // For pipes, ERROR_NO_DATA is not an error, but the pipe is closing. + return 0; - default: - throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + case Interop.Errors.ERROR_INVALID_PARAMETER: + // ERROR_INVALID_PARAMETER may be returned for writes + // where the position is too large or for synchronous writes + // to a handle opened asynchronously. + throw new IOException(SR.IO_FileTooLong); + + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode, path); + } + } + } + finally + { + if (overlapped != null) + { + resetEvent.FreeNativeOverlappedIfItIsSafe(overlapped); } + + resetEvent.Dispose(); } } @@ -595,18 +627,13 @@ private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, return new ValueTask(vts, vts.Version); } - private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(SafeFileHandle handle, long fileOffset, ManualResetEvent mres) + private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(ThreadPoolBoundHandle threadPoolBinding, long fileOffset, CallbackResetEvent resetEvent) { - Debug.Assert(handle.IsAsync); - - handle.EnsureThreadPoolBindingInitialized(); - // After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding // to allocate a native overlapped and provide a valid callback. // Since we really don't care about the callback (because this is sync IO for async handle) - // and we are going to wait on WaitHandle anyway, we pass ThreadPoolBinding as a state. - // The callback is going to use it to free the native overlapped. - NativeOverlapped* result = handle.ThreadPoolBinding!.AllocateNativeOverlapped(s_callback, handle.ThreadPoolBinding, null); + // and we are going to wait on WaitHandle anyway, we pass null as a state. + NativeOverlapped* result = threadPoolBinding.AllocateNativeOverlapped(s_callback, resetEvent, null); // For pipes the offsets are ignored by the OS result->OffsetLow = unchecked((int)fileOffset); @@ -619,7 +646,7 @@ private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, // are performed on the same file, named pipe, or communications device. // In this situation, there is no way to know which operation caused the object's state to be signaled." // Since we want RandomAccess APIs to be thread-safe, we provide a dedicated wait handle. - result->EventHandle = mres.SafeWaitHandle.DangerousGetHandle(); + result->EventHandle = resetEvent.SafeWaitHandle.DangerousGetHandle(); return result; } @@ -640,8 +667,34 @@ private static unsafe IOCompletionCallback AllocateCallback() static unsafe void Callback(uint errorCode, uint numBytes, NativeOverlapped* pOverlapped) { - ThreadPoolBoundHandle threadPoolBoundHandle = (ThreadPoolBoundHandle)ThreadPoolBoundHandle.GetNativeOverlappedState(pOverlapped)!; - threadPoolBoundHandle.FreeNativeOverlapped(pOverlapped); + CallbackResetEvent state = (CallbackResetEvent)ThreadPoolBoundHandle.GetNativeOverlappedState(pOverlapped)!; + state.FreeNativeOverlappedIfItIsSafe(pOverlapped); + } + } + + // We need to store the reference count (see the comment in FreeNativeOverlappedIfItIsSafe) and an EventHandle to signal the completition. + // We could keep these two things separate, but since ManualResetEvent is sealed and we want to avoid any extra allocations, this type has been created. + // It's basically ManualResetEvent with reference count. + private sealed class CallbackResetEvent : EventWaitHandle + { + private int _freeWhenZero = 2; // one for the callback and another for GetOverlappedResult + private ThreadPoolBoundHandle _threadPoolBoundHandle; + + internal CallbackResetEvent(bool initialState, ThreadPoolBoundHandle threadPoolBoundHandle) : base(initialState, EventResetMode.ManualReset) + { + _threadPoolBoundHandle = threadPoolBoundHandle; + } + + internal unsafe void FreeNativeOverlappedIfItIsSafe(NativeOverlapped* pOverlapped) + { + // Each SafeFileHandle opened for async IO is bound to ThreadPool. + // It requires us to provide a callback even if we want to observe the result by using GetOverlappedResult. + // There can be a race condition between GetOverlappedResult and the callback invocation, + // so we need to track the number of references, and when it drops to zero, then free the native overlapped. + if (Interlocked.Decrement(ref _freeWhenZero) == 0) + { + _threadPoolBoundHandle.FreeNativeOverlapped(pOverlapped); + } } } } From 1267d95f027b10c729b8e28f0a4f7003876ef71b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 30 Jun 2021 14:25:42 +0200 Subject: [PATCH 09/15] log last error in case of unexpected return value from Read|WriteFile methods --- .../src/System/IO/RandomAccess.Windows.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 302141908c74c8..480a6942205c46 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -74,9 +74,9 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span Date: Wed, 30 Jun 2021 17:14:58 +0200 Subject: [PATCH 10/15] handle case where WriteFile or ReadFille return 1 and LastError returns ERROR_SUCCESS --- .../src/System/IO/RandomAccess.Windows.cs | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 480a6942205c46..78f5021dc0db62 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -73,14 +73,17 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span Date: Wed, 30 Jun 2021 21:37:27 +0200 Subject: [PATCH 11/15] minor fixes after reading the code again --- .../src/System/IO/RandomAccess.Windows.cs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 78f5021dc0db62..2a120cb94cd3a6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -113,7 +113,7 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span WriteFileGatherAsync(SafeFileHandle handle, { // After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding // to allocate a native overlapped and provide a valid callback. - // Since we really don't care about the callback (because this is sync IO for async handle) - // and we are going to wait on WaitHandle anyway, we pass null as a state. NativeOverlapped* result = threadPoolBinding.AllocateNativeOverlapped(s_callback, resetEvent, null); // For pipes the offsets are ignored by the OS @@ -674,7 +672,7 @@ private static unsafe IOCompletionCallback AllocateCallback() static unsafe void Callback(uint errorCode, uint numBytes, NativeOverlapped* pOverlapped) { CallbackResetEvent state = (CallbackResetEvent)ThreadPoolBoundHandle.GetNativeOverlappedState(pOverlapped)!; - state.FreeNativeOverlappedIfItIsSafe(pOverlapped); + state.FreeNativeOverlapped(pOverlapped); } } @@ -683,7 +681,7 @@ static unsafe void Callback(uint errorCode, uint numBytes, NativeOverlapped* pOv // It's basically ManualResetEvent with reference count. private sealed class CallbackResetEvent : EventWaitHandle { - private int _freeWhenZero = 2; // one for the callback and another for GetOverlappedResult + private int _freeWhenZero = 2; // one for the callback and another for the method that calls GetOverlappedResult private ThreadPoolBoundHandle _threadPoolBoundHandle; internal CallbackResetEvent(bool initialState, ThreadPoolBoundHandle threadPoolBoundHandle) : base(initialState, EventResetMode.ManualReset) @@ -691,11 +689,11 @@ internal CallbackResetEvent(bool initialState, ThreadPoolBoundHandle threadPoolB _threadPoolBoundHandle = threadPoolBoundHandle; } - internal unsafe void FreeNativeOverlappedIfItIsSafe(NativeOverlapped* pOverlapped) + internal unsafe void FreeNativeOverlapped(NativeOverlapped* pOverlapped) { // Each SafeFileHandle opened for async IO is bound to ThreadPool. - // It requires us to provide a callback even if we want to observe the result by using GetOverlappedResult. - // There can be a race condition between GetOverlappedResult and the callback invocation, + // It requires us to provide a callback even if we want to use EventHandle and use GetOverlappedResult to obtain the result. + // There can be a race condition between the call to GetOverlappedResult and the callback invocation, // so we need to track the number of references, and when it drops to zero, then free the native overlapped. if (Interlocked.Decrement(ref _freeWhenZero) == 0) { From 4795fd3f7d688a342b6ee9e7ff8ff9c28c578557 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 1 Jul 2021 09:48:43 +0200 Subject: [PATCH 12/15] don't run async file IO tests on Mono on Windows, as it's not supported --- .../System.IO.FileSystem/tests/RandomAccess/Base.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs index 6642d6b4ca0eb6..de9a58b31d1799 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs @@ -18,7 +18,12 @@ public abstract class RandomAccess_Base : FileSystemTest public static IEnumerable GetSyncAsyncOptions() { yield return new object[] { FileOptions.None }; - yield return new object[] { FileOptions.Asynchronous }; + + // https://github.com/dotnet/runtime/issues/34582 + if (!(OperatingSystem.IsWindows && PlatformDetection.IsMonoRuntime)) + { + yield return new object[] { FileOptions.Asynchronous }; + } } [Fact] From 90ad6ecc8b7b769fa5a5b52eaf6179b78b0aaab4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 1 Jul 2021 11:01:27 +0200 Subject: [PATCH 13/15] add missing () --- src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs index de9a58b31d1799..82feb30d3e7e29 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs @@ -20,7 +20,7 @@ public static IEnumerable GetSyncAsyncOptions() yield return new object[] { FileOptions.None }; // https://github.com/dotnet/runtime/issues/34582 - if (!(OperatingSystem.IsWindows && PlatformDetection.IsMonoRuntime)) + if (!(OperatingSystem.IsWindows() && PlatformDetection.IsMonoRuntime)) { yield return new object[] { FileOptions.Asynchronous }; } From 475b1799cc16bf235fde3525aa5c7b40b91606de Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 2 Jul 2021 09:00:13 +0200 Subject: [PATCH 14/15] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/System/IO/RandomAccess.Windows.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 2a120cb94cd3a6..610af86a8d96a2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -64,7 +64,7 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span Date: Fri, 2 Jul 2021 13:01:48 +0200 Subject: [PATCH 15/15] dont run async file IO tests on configs that don't support async file IO --- .../TestUtilities/System/PlatformDetection.cs | 2 ++ .../FileStreamConformanceTests.Windows.cs | 3 ++- .../tests/FileStream/FileStreamOptions.cs | 22 ++++++++++++++----- .../tests/RandomAccess/Base.cs | 3 +-- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs index aa44cfe53f1b02..fc829692c03f47 100644 --- a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs +++ b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs @@ -91,6 +91,8 @@ public static bool IsDrawingSupported } } + public static bool IsAsyncFileIOSupported => !IsBrowser && !(IsWindows && IsMonoRuntime); // https://github.com/dotnet/runtime/issues/34582 + public static bool IsLineNumbersSupported => true; public static bool IsInContainer => GetIsInContainer(); diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.Windows.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.Windows.cs index b6b2be09ad54db..a372213550fa81 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.Windows.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.Windows.cs @@ -42,6 +42,7 @@ protected override async Task CreateConnectedStreamsAsync() } [PlatformSpecific(TestPlatforms.Windows)] // DOS device paths (\\.\ and \\?\) are a Windows concept + [ActiveIssue("https://github.com/dotnet/runtime/issues/34582", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] public class SeekableDeviceFileStreamStandaloneConformanceTests : UnbufferedAsyncFileStreamStandaloneConformanceTests { protected override string GetTestFilePath(int? index = null, [CallerMemberName] string memberName = null, [CallerLineNumber] int lineNumber = 0) @@ -90,7 +91,7 @@ public class UncFilePathFileStreamStandaloneConformanceTests : UnbufferedAsyncFi return false; } - // the "Server Service" allows for file sharing. It can be disabled on some of our CI machines. + // the "Server Service" allows for file sharing. It can be disabled on some of our CI machines. using (ServiceController sharingService = new ServiceController("Server")) { return sharingService.Status == ServiceControllerStatus.Running; diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamOptions.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamOptions.cs index 9be492d21bdbf9..8dd7f87b84340a 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamOptions.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamOptions.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Linq; using System.Text; using Xunit; @@ -135,13 +136,22 @@ public void BufferSize() Assert.Throws(() => new FileStreamOptions { BufferSize = -1 }); } + public static IEnumerable GetSettingsArePropagatedArguments() + { + yield return new object[] { FileMode.Create, FileAccess.Write, FileOptions.None }; + yield return new object[] { FileMode.Open, FileAccess.Read, FileOptions.None }; + yield return new object[] { FileMode.Create, FileAccess.ReadWrite, FileOptions.None }; + + if (PlatformDetection.IsAsyncFileIOSupported) + { + yield return new object[] { FileMode.Create, FileAccess.Write, FileOptions.Asynchronous }; + yield return new object[] { FileMode.Open, FileAccess.Read, FileOptions.Asynchronous }; + yield return new object[] { FileMode.Create, FileAccess.ReadWrite, FileOptions.Asynchronous }; + } + } + [Theory] - [InlineData(FileMode.Create, FileAccess.Write, FileOptions.None)] - [InlineData(FileMode.Create, FileAccess.Write, FileOptions.Asynchronous)] - [InlineData(FileMode.Open, FileAccess.Read, FileOptions.None)] - [InlineData(FileMode.Open, FileAccess.Read, FileOptions.Asynchronous)] - [InlineData(FileMode.Create, FileAccess.ReadWrite, FileOptions.None)] - [InlineData(FileMode.Create, FileAccess.ReadWrite, FileOptions.Asynchronous)] + [MemberData(nameof(GetSettingsArePropagatedArguments))] public void SettingsArePropagated(FileMode mode, FileAccess access, FileOptions fileOptions) { string filePath = GetTestFilePath(); diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs index 82feb30d3e7e29..6f8726500d3b3b 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/Base.cs @@ -19,8 +19,7 @@ public static IEnumerable GetSyncAsyncOptions() { yield return new object[] { FileOptions.None }; - // https://github.com/dotnet/runtime/issues/34582 - if (!(OperatingSystem.IsWindows() && PlatformDetection.IsMonoRuntime)) + if (PlatformDetection.IsAsyncFileIOSupported) { yield return new object[] { FileOptions.Asynchronous }; }