From 6e028ed15e2dab4b1f80ec033fc870f9c38549a4 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Thu, 30 Sep 2021 16:44:13 -0700 Subject: [PATCH 01/13] Fallback to read/write if pread/pwrite fails --- .../tests/FileStream/CharacterDevice.cs | 32 +++++++++++++++++++ .../tests/System.IO.FileSystem.Tests.csproj | 1 + .../src/System/IO/RandomAccess.Unix.cs | 24 ++++++++++---- 3 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs new file mode 100644 index 0000000000000..a8a3058c251fb --- /dev/null +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs @@ -0,0 +1,32 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace System.IO.Tests +{ + [PlatformSpecific(TestPlatforms.AnyUnix)] + public class CharacterDevice + { + const string CharacterDevicePath = "/dev/tty"; + [Theory] + [InlineData(FileOptions.None)] + [InlineData(FileOptions.Asynchronous)] + public void CharacterDevice_Write(FileOptions fileOptions) + { + using FileStream fs = new(CharacterDevicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); + fs.Write(Encoding.UTF8.GetBytes("foo")); + } + + [Theory] + [InlineData(FileOptions.None)] + [InlineData(FileOptions.Asynchronous)] + public async Task CharacterDevice_WriteAsync(FileOptions fileOptions) + { + using FileStream fs = new(CharacterDevicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); + await fs.WriteAsync(Encoding.UTF8.GetBytes("foo")); + } + } +} 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 a9bedb8bd312b..6ad26a9f3a6c7 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 @@ -114,6 +114,7 @@ + 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 992dcc54f5be5..f0c99a70316c6 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 @@ -33,9 +33,15 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer // The Windows implementation uses ReadFile, which ignores the offset if the handle // isn't seekable. We do the same manually with PRead vs Read, in order to enable // the function to be used by FileStream for all the same situations. - int result = handle.CanSeek ? - Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset) : - Interop.Sys.Read(handle, bufPtr, buffer.Length); + int result = Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset); + + // Fallback to read if pread fails. + if (result == -1) + { + Debug.Assert(Interop.Sys.GetLastErrorInfo().Error == Interop.Error.ENXIO || !handle.CanSeek); // We want to catch other errors and add unit tests for them. + result = Interop.Sys.Read(handle, bufPtr, buffer.Length); + } + FileStreamHelpers.CheckFileCall(result, handle.Path); return result; } @@ -90,9 +96,15 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan Date: Sun, 3 Oct 2021 17:15:09 -0700 Subject: [PATCH 02/13] * Cache whether a handle supports random access * Keep avoiding pread/pwrite for unseekable files * Fallback to read/write only for known error ENXIO --- .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 7 +++ .../src/System/IO/RandomAccess.Unix.cs | 48 +++++++++++++++---- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index 777997444848f..757f4e36963c4 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -33,6 +33,13 @@ private SafeFileHandle(bool ownsHandle) internal bool CanSeek => !IsClosed && GetCanSeek(); + private bool? _supportsRandomAccess; + internal bool SupportsRandomAccess + { + get => _supportsRandomAccess ??= CanSeek; + set => _supportsRandomAccess = value; + } + internal ThreadPoolBoundHandle? ThreadPoolBinding => null; internal void EnsureThreadPoolBindingInitialized() { /* nop */ } 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 f0c99a70316c6..87b06960567fd 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 @@ -33,12 +33,27 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer // The Windows implementation uses ReadFile, which ignores the offset if the handle // isn't seekable. We do the same manually with PRead vs Read, in order to enable // the function to be used by FileStream for all the same situations. - int result = Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset); - - // Fallback to read if pread fails. - if (result == -1) + int result; + if (handle.SupportsRandomAccess) + { + // Try pread for seekable files. + result = Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset); + if (result == -1) + { + // Some devices (such as /dev/tty) are reported as seekable by macOS but pread is unable to handle them and returns ENXIO. + // Historically we were able to handle /dev/tty using read so we need to fallback to read for that case. + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + // We want to discover more errors that could make pread fail and add unit tests for them. + Debug.Assert(errorInfo.Error == Interop.Error.ENXIO); + if (errorInfo.Error == Interop.Error.ENXIO) + { + handle.SupportsRandomAccess = false; + result = Interop.Sys.Read(handle, bufPtr, buffer.Length); + } + } + } + else { - Debug.Assert(Interop.Sys.GetLastErrorInfo().Error == Interop.Error.ENXIO || !handle.CanSeek); // We want to catch other errors and add unit tests for them. result = Interop.Sys.Read(handle, bufPtr, buffer.Length); } @@ -97,12 +112,27 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan Date: Sun, 3 Oct 2021 17:16:33 -0700 Subject: [PATCH 03/13] Test more device paths and add tests for WriteAll* --- .../tests/FileStream/CharacterDevice.cs | 80 ++++++++++++++++--- 1 file changed, 71 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs index a8a3058c251fb..ee1ddb1b451a8 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs @@ -1,8 +1,10 @@ // 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.Text; using System.Threading.Tasks; +using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace System.IO.Tests @@ -10,23 +12,83 @@ namespace System.IO.Tests [PlatformSpecific(TestPlatforms.AnyUnix)] public class CharacterDevice { - const string CharacterDevicePath = "/dev/tty"; [Theory] - [InlineData(FileOptions.None)] - [InlineData(FileOptions.Asynchronous)] - public void CharacterDevice_Write(FileOptions fileOptions) + [MemberData(nameof(DevicePath_FileOptions_TestData))] + public void CharacterDevice_FileStream_Write(string devicePath, FileOptions fileOptions) { - using FileStream fs = new(CharacterDevicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); + VerifyDeviceExists(devicePath); + using FileStream fs = new(devicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); fs.Write(Encoding.UTF8.GetBytes("foo")); } [Theory] - [InlineData(FileOptions.None)] - [InlineData(FileOptions.Asynchronous)] - public async Task CharacterDevice_WriteAsync(FileOptions fileOptions) + [MemberData(nameof(DevicePath_FileOptions_TestData))] + public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileOptions fileOptions) { - using FileStream fs = new(CharacterDevicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); + VerifyDeviceExists(devicePath); + using FileStream fs = new(devicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); await fs.WriteAsync(Encoding.UTF8.GetBytes("foo")); } + + [Theory] + [MemberData(nameof(DevicePath_TestData))] + public void CharacterDevice_WriteAllBytes(string devicePath) + { + VerifyDeviceExists(devicePath); + File.WriteAllBytes(devicePath, Encoding.UTF8.GetBytes("foo")); + } + + [Theory] + [MemberData(nameof(DevicePath_TestData))] + public async Task CharacterDevice_WriteAllBytesAsync(string devicePath) + { + VerifyDeviceExists(devicePath); + await File.WriteAllBytesAsync(devicePath, Encoding.UTF8.GetBytes("foo")); + } + + [Theory] + [MemberData(nameof(DevicePath_TestData))] + public void CharacterDevice_WriteAllText(string devicePath) + { + VerifyDeviceExists(devicePath); + File.WriteAllText(devicePath, "foo"); + } + + [Theory] + [MemberData(nameof(DevicePath_TestData))] + public async Task CharacterDevice_WriteAllTextAsync(string devicePath) + { + VerifyDeviceExists(devicePath); + await File.WriteAllTextAsync(devicePath, "foo"); + } + + private static void VerifyDeviceExists(string devicePath) + { + if (!File.Exists(devicePath)) + { + throw new SkipTestException("Device does not exists in this platform"); + } + } + + private static string[] DevicePaths = { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }; + + public static IEnumerable DevicePath_FileOptions_TestData() + { + foreach (string devicePath in DevicePaths) + { + foreach (FileOptions options in Enum.GetValues()) + { + yield return new object[] { devicePath, options}; + } + } + } + + public static IEnumerable DevicePath_TestData() + { + foreach (string devicePath in DevicePaths) + { + yield return new object[] { devicePath }; + } + } } } From f2e90a54a39992a03abef8eb876bcf9ddf5aeb29 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Sun, 3 Oct 2021 21:44:24 -0700 Subject: [PATCH 04/13] Fix CI: skip test when device can't be opened --- .../tests/FileStream/CharacterDevice.cs | 39 +++++++++++++------ .../src/System/IO/RandomAccess.Unix.cs | 4 +- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs index ee1ddb1b451a8..628a884b35653 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs @@ -16,8 +16,10 @@ public class CharacterDevice [MemberData(nameof(DevicePath_FileOptions_TestData))] public void CharacterDevice_FileStream_Write(string devicePath, FileOptions fileOptions) { - VerifyDeviceExists(devicePath); - using FileStream fs = new(devicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); + FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write }; + VerifyDeviceIsReachable(devicePath, options); + + using FileStream fs = new(devicePath, options); fs.Write(Encoding.UTF8.GetBytes("foo")); } @@ -25,8 +27,10 @@ public void CharacterDevice_FileStream_Write(string devicePath, FileOptions file [MemberData(nameof(DevicePath_FileOptions_TestData))] public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileOptions fileOptions) { - VerifyDeviceExists(devicePath); - using FileStream fs = new(devicePath, new FileStreamOptions { Options = fileOptions, Access = FileAccess.Write }); + FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write }; + VerifyDeviceIsReachable(devicePath, options); + + using FileStream fs = new(devicePath, options); await fs.WriteAsync(Encoding.UTF8.GetBytes("foo")); } @@ -34,7 +38,8 @@ public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileO [MemberData(nameof(DevicePath_TestData))] public void CharacterDevice_WriteAllBytes(string devicePath) { - VerifyDeviceExists(devicePath); + VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write }); + File.WriteAllBytes(devicePath, Encoding.UTF8.GetBytes("foo")); } @@ -42,7 +47,8 @@ public void CharacterDevice_WriteAllBytes(string devicePath) [MemberData(nameof(DevicePath_TestData))] public async Task CharacterDevice_WriteAllBytesAsync(string devicePath) { - VerifyDeviceExists(devicePath); + VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write }); + await File.WriteAllBytesAsync(devicePath, Encoding.UTF8.GetBytes("foo")); } @@ -50,7 +56,8 @@ public async Task CharacterDevice_WriteAllBytesAsync(string devicePath) [MemberData(nameof(DevicePath_TestData))] public void CharacterDevice_WriteAllText(string devicePath) { - VerifyDeviceExists(devicePath); + VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write }); + File.WriteAllText(devicePath, "foo"); } @@ -58,16 +65,26 @@ public void CharacterDevice_WriteAllText(string devicePath) [MemberData(nameof(DevicePath_TestData))] public async Task CharacterDevice_WriteAllTextAsync(string devicePath) { - VerifyDeviceExists(devicePath); + VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write }); + await File.WriteAllTextAsync(devicePath, "foo"); } - private static void VerifyDeviceExists(string devicePath) + private static void VerifyDeviceIsReachable(string devicePath, FileStreamOptions? options) { if (!File.Exists(devicePath)) { - throw new SkipTestException("Device does not exists in this platform"); - } + throw new SkipTestException("Device does not exists"); + } + + try + { + File.Open(devicePath, options).Dispose(); + } + catch (IOException ex) + { + throw new SkipTestException($"Device failed to open: {ex.Message}"); + } } private static string[] DevicePaths = { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }; 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 87b06960567fd..17049ec0f04de 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 @@ -44,7 +44,7 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer // Historically we were able to handle /dev/tty using read so we need to fallback to read for that case. Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); // We want to discover more errors that could make pread fail and add unit tests for them. - Debug.Assert(errorInfo.Error == Interop.Error.ENXIO); + Debug.Assert(errorInfo.Error == Interop.Error.ENXIO, $"Unexpected error: {errorInfo.Error}"); if (errorInfo.Error == Interop.Error.ENXIO) { handle.SupportsRandomAccess = false; @@ -122,7 +122,7 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan Date: Sun, 3 Oct 2021 23:19:46 -0700 Subject: [PATCH 05/13] * Do not use SkipTestException * Exclude EBADF from assertion --- .../tests/FileStream/CharacterDevice.cs | 42 ++++++++++++++----- .../src/System/IO/RandomAccess.Unix.cs | 14 +++++-- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs index 628a884b35653..f7cf2171f7804 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs @@ -17,7 +17,10 @@ public class CharacterDevice public void CharacterDevice_FileStream_Write(string devicePath, FileOptions fileOptions) { FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write }; - VerifyDeviceIsReachable(devicePath, options); + if (IsDeviceUnreachable(devicePath, options)) + { + return; + } using FileStream fs = new(devicePath, options); fs.Write(Encoding.UTF8.GetBytes("foo")); @@ -28,7 +31,10 @@ public void CharacterDevice_FileStream_Write(string devicePath, FileOptions file public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileOptions fileOptions) { FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write }; - VerifyDeviceIsReachable(devicePath, options); + if (IsDeviceUnreachable(devicePath, options)) + { + return; + } using FileStream fs = new(devicePath, options); await fs.WriteAsync(Encoding.UTF8.GetBytes("foo")); @@ -38,7 +44,10 @@ public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileO [MemberData(nameof(DevicePath_TestData))] public void CharacterDevice_WriteAllBytes(string devicePath) { - VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write }); + if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write })) + { + return; + } File.WriteAllBytes(devicePath, Encoding.UTF8.GetBytes("foo")); } @@ -47,7 +56,10 @@ public void CharacterDevice_WriteAllBytes(string devicePath) [MemberData(nameof(DevicePath_TestData))] public async Task CharacterDevice_WriteAllBytesAsync(string devicePath) { - VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write }); + if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write })) + { + return; + } await File.WriteAllBytesAsync(devicePath, Encoding.UTF8.GetBytes("foo")); } @@ -56,7 +68,10 @@ public async Task CharacterDevice_WriteAllBytesAsync(string devicePath) [MemberData(nameof(DevicePath_TestData))] public void CharacterDevice_WriteAllText(string devicePath) { - VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write }); + if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write })) + { + return; + } File.WriteAllText(devicePath, "foo"); } @@ -65,26 +80,31 @@ public void CharacterDevice_WriteAllText(string devicePath) [MemberData(nameof(DevicePath_TestData))] public async Task CharacterDevice_WriteAllTextAsync(string devicePath) { - VerifyDeviceIsReachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write }); + if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write })) + { + return; + } await File.WriteAllTextAsync(devicePath, "foo"); } - private static void VerifyDeviceIsReachable(string devicePath, FileStreamOptions? options) + private static bool IsDeviceUnreachable(string devicePath, FileStreamOptions? options) { if (!File.Exists(devicePath)) { - throw new SkipTestException("Device does not exists"); + return true; } try { File.Open(devicePath, options).Dispose(); } - catch (IOException ex) + catch (IOException) { - throw new SkipTestException($"Device failed to open: {ex.Message}"); + return true; } + + return false; } private static string[] DevicePaths = { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }; @@ -93,7 +113,7 @@ public static IEnumerable DevicePath_FileOptions_TestData() { foreach (string devicePath in DevicePaths) { - foreach (FileOptions options in Enum.GetValues()) + foreach (FileOptions options in new[] { FileOptions.None, FileOptions.Asynchronous }) { yield return new object[] { devicePath, options}; } 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 17049ec0f04de..5f37deb94bd2b 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 @@ -43,8 +43,8 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer // Some devices (such as /dev/tty) are reported as seekable by macOS but pread is unable to handle them and returns ENXIO. // Historically we were able to handle /dev/tty using read so we need to fallback to read for that case. Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - // We want to discover more errors that could make pread fail and add unit tests for them. - Debug.Assert(errorInfo.Error == Interop.Error.ENXIO, $"Unexpected error: {errorInfo.Error}"); + AssertPReadPWriteKnownErrors(errorInfo.Error); + if (errorInfo.Error == Interop.Error.ENXIO) { handle.SupportsRandomAccess = false; @@ -121,8 +121,7 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan> buffers, long fileOffset) { int buffersCount = buffers.Count; From 70938956d1bd3546a540df6a7cdbd582cf49b09e Mon Sep 17 00:00:00 2001 From: Jozkee Date: Mon, 4 Oct 2021 00:00:27 -0700 Subject: [PATCH 06/13] use NullableBool in _supportsRandomAccess --- .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index 757f4e36963c4..47a676105c05b 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -16,6 +16,7 @@ public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid // not using bool? as it's not thread safe private volatile NullableBool _canSeek = NullableBool.Undefined; + private volatile NullableBool _supportsRandomAccess = NullableBool.Undefined; private bool _deleteOnClose; private bool _isLocked; @@ -33,11 +34,18 @@ private SafeFileHandle(bool ownsHandle) internal bool CanSeek => !IsClosed && GetCanSeek(); - private bool? _supportsRandomAccess; internal bool SupportsRandomAccess { - get => _supportsRandomAccess ??= CanSeek; - set => _supportsRandomAccess = value; + get + { + if (_supportsRandomAccess == NullableBool.Undefined) + { + _supportsRandomAccess = GetCanSeek() ? NullableBool.True : NullableBool.False; + } + + return _supportsRandomAccess == NullableBool.True; + } + set => _supportsRandomAccess = value ? NullableBool.True : NullableBool.False; } internal ThreadPoolBoundHandle? ThreadPoolBinding => null; From 1a76ae5740edc47f624952b37d5920a0ab8c45a9 Mon Sep 17 00:00:00 2001 From: Jozkee Date: Tue, 5 Oct 2021 18:43:51 -0700 Subject: [PATCH 07/13] Add tests for mkfifo and socketpair --- .../tests/FileStream/CharacterDevice.cs | 130 +++++++++++++++++- 1 file changed, 126 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs index f7cf2171f7804..839f9bf2a6013 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs @@ -1,16 +1,19 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.DotNet.XUnitExtensions; +using Microsoft.Win32.SafeHandles; using System.Collections.Generic; +using System.IO.Pipes; +using System.Runtime.InteropServices; using System.Text; using System.Threading.Tasks; -using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace System.IO.Tests { [PlatformSpecific(TestPlatforms.AnyUnix)] - public class CharacterDevice + public class CharacterDevice : FileSystemTest { [Theory] [MemberData(nameof(DevicePath_FileOptions_TestData))] @@ -88,6 +91,114 @@ public async Task CharacterDevice_WriteAllTextAsync(string devicePath) await File.WriteAllTextAsync(devicePath, "foo"); } + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] + public async Task NamedPipe_ReadWrite() + { + string fifoPath = GetTestFilePath(); + Assert.Equal(0, mkfifo(fifoPath, 438 /* 666 in octal */ )); + + await Task.WhenAll( + Task.Run(() => + { + using var fs = File.OpenRead(fifoPath); + ReadByte(fs, 42); + }), + Task.Run(() => + { + using var fs = File.OpenWrite(fifoPath); + WriteByte(fs, 42); + })); + } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] + public async Task NamedPipe_ReadWrite_Async() + { + string fifoPath = GetTestFilePath(); + Assert.Equal(0, mkfifo(fifoPath, 438 /* 666 in octal */ )); + + await Task.WhenAll( + Task.Run(async () => { + using var fs = File.OpenRead(fifoPath); + await ReadByteAsync(fs, 42); + }), + Task.Run(async () => + { + using var fs = File.OpenWrite(fifoPath); + await WriteByteAsync(fs, 42); + })); + } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] + public unsafe void SocketPair_ReadWrite() + { + const int AF_UNIX = 1; + const int SOCK_STREAM = 1; + int* ptr = stackalloc int[2]; + Assert.Equal(0, socketpair(AF_UNIX, SOCK_STREAM, 0, ptr)); + + using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: false), FileAccess.Read); + using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: false), FileAccess.Write); + + Task.WhenAll( + Task.Run(() => ReadByte(readFileStream, 42)), + Task.Run(() => WriteByte(writeFileStream, 42))).GetAwaiter().GetResult(); + + close(ptr[0]); + close(ptr[1]); + } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] + public void SocketPair_ReadWrite_Async() + { + const int AF_UNIX = 1; + const int SOCK_STREAM = 1; + unsafe + { + int* ptr = stackalloc int[2]; + Assert.Equal(0, socketpair(AF_UNIX, SOCK_STREAM, 0, ptr)); + + using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: false), FileAccess.Read); + using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: false), FileAccess.Write); + + Task.WhenAll( + ReadByteAsync(readFileStream, 42), + WriteByteAsync(writeFileStream, 42)).GetAwaiter().GetResult(); + + close(ptr[0]); + close(ptr[1]); + } + } + + private static void ReadByte(FileStream fs, byte expected) + { + var buffer = new byte[1]; + Assert.Equal(1, fs.Read(buffer)); + Assert.Equal(expected, buffer[0]); + } + + private static void WriteByte(FileStream fs, byte value) + { + fs.Write(new byte[] { value }); + fs.Flush(); + } + + private static async Task ReadByteAsync(FileStream fs, byte expected) + { + var buffer = new byte[1]; + Assert.Equal(1, await fs.ReadAsync(buffer)); + Assert.Equal(expected, buffer[0]); + } + + private static async Task WriteByteAsync(FileStream fs, byte value) + { + await fs.WriteAsync(new byte[] { value }); + await fs.FlushAsync(); + } + private static bool IsDeviceUnreachable(string devicePath, FileStreamOptions? options) { if (!File.Exists(devicePath)) @@ -99,9 +210,14 @@ private static bool IsDeviceUnreachable(string devicePath, FileStreamOptions? op { File.Open(devicePath, options).Dispose(); } - catch (IOException) + catch (Exception ex) { - return true; + if (ex is IOException || ex is UnauthorizedAccessException) + { + return true; + } + + throw; } return false; @@ -127,5 +243,11 @@ public static IEnumerable DevicePath_TestData() yield return new object[] { devicePath }; } } + + [DllImport("libc")] + private static unsafe extern int socketpair(int domain, int type, int protocol, int* ptr); + + [DllImport("libc")] + private static extern int close(int fd); } } From f358e1ff634292fa6ac3c611ee698553427ca8ec Mon Sep 17 00:00:00 2001 From: Jozkee Date: Tue, 5 Oct 2021 19:10:34 -0700 Subject: [PATCH 08/13] Rename test file --- .../{CharacterDevice.cs => DevicesPipesAndSockets.cs} | 2 +- .../tests/System.IO.FileSystem.Tests.csproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename src/libraries/System.IO.FileSystem/tests/FileStream/{CharacterDevice.cs => DevicesPipesAndSockets.cs} (99%) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs similarity index 99% rename from src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs rename to src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs index 839f9bf2a6013..848e894c4e88f 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/CharacterDevice.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs @@ -13,7 +13,7 @@ namespace System.IO.Tests { [PlatformSpecific(TestPlatforms.AnyUnix)] - public class CharacterDevice : FileSystemTest + public class DevicesPipesAndSockets : FileSystemTest { [Theory] [MemberData(nameof(DevicePath_FileOptions_TestData))] 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 6ad26a9f3a6c7..f5ee36a10d84c 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 @@ -114,7 +114,7 @@ - + From e569ecdd7d14e0401ca836c7ccdf0c023a97bd46 Mon Sep 17 00:00:00 2001 From: Jozkee Date: Tue, 5 Oct 2021 19:18:27 -0700 Subject: [PATCH 09/13] Address feedback in src --- .../src/System/IO/RandomAccess.Unix.cs | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) 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 5f37deb94bd2b..bdbda61624f91 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 @@ -40,12 +40,12 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer result = Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset); if (result == -1) { - // Some devices (such as /dev/tty) are reported as seekable by macOS but pread is unable to handle them and returns ENXIO. - // Historically we were able to handle /dev/tty using read so we need to fallback to read for that case. + // We need to fallback to the non-offset version for certain file types + // e.g: character devices (such as /dev/tty), pipes, and sockets. Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - AssertPReadPWriteKnownErrors(errorInfo.Error); - if (errorInfo.Error == Interop.Error.ENXIO) + if (errorInfo.Error == Interop.Error.ENXIO || + errorInfo.Error == Interop.Error.ESPIPE) { handle.SupportsRandomAccess = false; result = Interop.Sys.Read(handle, bufPtr, buffer.Length); @@ -118,12 +118,12 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan> buffers, long fileOffset) { int buffersCount = buffers.Count; From 7c73f83b0f5551dead2c2c5c9d5871dbb648b373 Mon Sep 17 00:00:00 2001 From: Jozkee Date: Thu, 7 Oct 2021 09:07:05 -0700 Subject: [PATCH 10/13] Address suggestions --- .../FileStream/DevicesPipesAndSockets.cs | 25 ++++++------------- .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 7 +++--- .../src/System/IO/RandomAccess.Unix.cs | 4 +-- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs index 848e894c4e88f..128df38d0235d 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs @@ -101,12 +101,12 @@ public async Task NamedPipe_ReadWrite() await Task.WhenAll( Task.Run(() => { - using var fs = File.OpenRead(fifoPath); + using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); ReadByte(fs, 42); }), Task.Run(() => { - using var fs = File.OpenWrite(fifoPath); + using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read); WriteByte(fs, 42); })); } @@ -120,12 +120,12 @@ public async Task NamedPipe_ReadWrite_Async() await Task.WhenAll( Task.Run(async () => { - using var fs = File.OpenRead(fifoPath); + using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); await ReadByteAsync(fs, 42); }), Task.Run(async () => { - using var fs = File.OpenWrite(fifoPath); + using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read); await WriteByteAsync(fs, 42); })); } @@ -139,15 +139,12 @@ public unsafe void SocketPair_ReadWrite() int* ptr = stackalloc int[2]; Assert.Equal(0, socketpair(AF_UNIX, SOCK_STREAM, 0, ptr)); - using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: false), FileAccess.Read); - using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: false), FileAccess.Write); + using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: true), FileAccess.Read); + using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: true), FileAccess.Write); Task.WhenAll( Task.Run(() => ReadByte(readFileStream, 42)), Task.Run(() => WriteByte(writeFileStream, 42))).GetAwaiter().GetResult(); - - close(ptr[0]); - close(ptr[1]); } [Fact] @@ -161,15 +158,12 @@ public void SocketPair_ReadWrite_Async() int* ptr = stackalloc int[2]; Assert.Equal(0, socketpair(AF_UNIX, SOCK_STREAM, 0, ptr)); - using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: false), FileAccess.Read); - using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: false), FileAccess.Write); + using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: true), FileAccess.Read); + using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: true), FileAccess.Write); Task.WhenAll( ReadByteAsync(readFileStream, 42), WriteByteAsync(writeFileStream, 42)).GetAwaiter().GetResult(); - - close(ptr[0]); - close(ptr[1]); } } @@ -246,8 +240,5 @@ public static IEnumerable DevicePath_TestData() [DllImport("libc")] private static unsafe extern int socketpair(int domain, int type, int protocol, int* ptr); - - [DllImport("libc")] - private static extern int close(int fd); } } diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index 47a676105c05b..6341d86f81870 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -38,12 +38,13 @@ internal bool SupportsRandomAccess { get { - if (_supportsRandomAccess == NullableBool.Undefined) + NullableBool supportsRandomAccess = _supportsRandomAccess; + if (supportsRandomAccess == NullableBool.Undefined) { - _supportsRandomAccess = GetCanSeek() ? NullableBool.True : NullableBool.False; + _supportsRandomAccess = supportsRandomAccess = GetCanSeek() ? NullableBool.True : NullableBool.False; } - return _supportsRandomAccess == NullableBool.True; + return supportsRandomAccess == NullableBool.True; } set => _supportsRandomAccess = value ? NullableBool.True : NullableBool.False; } 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 bdbda61624f91..d922e89c13de5 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 @@ -44,8 +44,8 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span buffer // e.g: character devices (such as /dev/tty), pipes, and sockets. Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.ENXIO || - errorInfo.Error == Interop.Error.ESPIPE) + if (errorInfo.Error == Interop.Error.ENXIO || + errorInfo.Error == Interop.Error.ESPIPE) { handle.SupportsRandomAccess = false; result = Interop.Sys.Read(handle, bufPtr, buffer.Length); From 565e2ab135a3cbb37a8c949d9728728a8cc02207 Mon Sep 17 00:00:00 2001 From: Jozkee Date: Fri, 8 Oct 2021 11:40:37 -0700 Subject: [PATCH 11/13] Address suggestions --- .../FileStream/DevicesPipesAndSockets.cs | 91 +++++++------------ .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 6 +- 2 files changed, 40 insertions(+), 57 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs index 128df38d0235d..63e7bcb2a2bf2 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs @@ -19,12 +19,7 @@ public class DevicesPipesAndSockets : FileSystemTest [MemberData(nameof(DevicePath_FileOptions_TestData))] public void CharacterDevice_FileStream_Write(string devicePath, FileOptions fileOptions) { - FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write }; - if (IsDeviceUnreachable(devicePath, options)) - { - return; - } - + FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write, Share = FileShare.Write }; using FileStream fs = new(devicePath, options); fs.Write(Encoding.UTF8.GetBytes("foo")); } @@ -33,12 +28,7 @@ public void CharacterDevice_FileStream_Write(string devicePath, FileOptions file [MemberData(nameof(DevicePath_FileOptions_TestData))] public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileOptions fileOptions) { - FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write }; - if (IsDeviceUnreachable(devicePath, options)) - { - return; - } - + FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write, Share = FileShare.Write }; using FileStream fs = new(devicePath, options); await fs.WriteAsync(Encoding.UTF8.GetBytes("foo")); } @@ -47,11 +37,6 @@ public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileO [MemberData(nameof(DevicePath_TestData))] public void CharacterDevice_WriteAllBytes(string devicePath) { - if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write })) - { - return; - } - File.WriteAllBytes(devicePath, Encoding.UTF8.GetBytes("foo")); } @@ -59,11 +44,6 @@ public void CharacterDevice_WriteAllBytes(string devicePath) [MemberData(nameof(DevicePath_TestData))] public async Task CharacterDevice_WriteAllBytesAsync(string devicePath) { - if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write })) - { - return; - } - await File.WriteAllBytesAsync(devicePath, Encoding.UTF8.GetBytes("foo")); } @@ -71,11 +51,6 @@ public async Task CharacterDevice_WriteAllBytesAsync(string devicePath) [MemberData(nameof(DevicePath_TestData))] public void CharacterDevice_WriteAllText(string devicePath) { - if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write })) - { - return; - } - File.WriteAllText(devicePath, "foo"); } @@ -83,11 +58,6 @@ public void CharacterDevice_WriteAllText(string devicePath) [MemberData(nameof(DevicePath_TestData))] public async Task CharacterDevice_WriteAllTextAsync(string devicePath) { - if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write })) - { - return; - } - await File.WriteAllTextAsync(devicePath, "foo"); } @@ -130,12 +100,13 @@ await Task.WhenAll( })); } + private const int AF_UNIX = 1; + private const int SOCK_STREAM = 1; + [Fact] [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] public unsafe void SocketPair_ReadWrite() { - const int AF_UNIX = 1; - const int SOCK_STREAM = 1; int* ptr = stackalloc int[2]; Assert.Equal(0, socketpair(AF_UNIX, SOCK_STREAM, 0, ptr)); @@ -151,8 +122,6 @@ public unsafe void SocketPair_ReadWrite() [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)] public void SocketPair_ReadWrite_Async() { - const int AF_UNIX = 1; - const int SOCK_STREAM = 1; unsafe { int* ptr = stackalloc int[2]; @@ -193,35 +162,45 @@ private static async Task WriteByteAsync(FileStream fs, byte value) await fs.FlushAsync(); } - private static bool IsDeviceUnreachable(string devicePath, FileStreamOptions? options) - { - if (!File.Exists(devicePath)) + private static List _devicePathsAvailable; + private static List GetDevicePaths() + { + if (_devicePathsAvailable is null) { - return true; - } + var options = new FileStreamOptions { Access = FileAccess.Write, Share = FileShare.Write }; + _devicePathsAvailable = new List(); - try - { - File.Open(devicePath, options).Dispose(); - } - catch (Exception ex) - { - if (ex is IOException || ex is UnauthorizedAccessException) + foreach (string devicePath in new[] { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }) { - return true; + if (!File.Exists(devicePath)) + { + continue; + } + + try + { + File.Open(devicePath, options).Dispose(); + } + catch (Exception ex) + { + if (ex is IOException || ex is UnauthorizedAccessException) + { + continue; + } + + throw; + } + + _devicePathsAvailable.Add(devicePath); } - - throw; } - return false; + return _devicePathsAvailable; } - private static string[] DevicePaths = { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }; - public static IEnumerable DevicePath_FileOptions_TestData() { - foreach (string devicePath in DevicePaths) + foreach (string devicePath in GetDevicePaths()) { foreach (FileOptions options in new[] { FileOptions.None, FileOptions.Asynchronous }) { @@ -232,7 +211,7 @@ public static IEnumerable DevicePath_FileOptions_TestData() public static IEnumerable DevicePath_TestData() { - foreach (string devicePath in DevicePaths) + foreach (string devicePath in GetDevicePaths()) { yield return new object[] { devicePath }; } diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index 6341d86f81870..9e26b95071a1c 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -46,7 +46,11 @@ internal bool SupportsRandomAccess return supportsRandomAccess == NullableBool.True; } - set => _supportsRandomAccess = value ? NullableBool.True : NullableBool.False; + set + { + Debug.Assert(value == false); // We should only use the setter to disable random access. + _supportsRandomAccess = value ? NullableBool.True : NullableBool.False; + } } internal ThreadPoolBoundHandle? ThreadPoolBinding => null; From 4378af051b79cc313fb208ff67bfc8052d68bd08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Mon, 11 Oct 2021 10:24:50 -0700 Subject: [PATCH 12/13] Update src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs Use Lazy for thread-safety Co-authored-by: Adam Sitnik --- .../FileStream/DevicesPipesAndSockets.cs | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs index 63e7bcb2a2bf2..182d8c9e51c18 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs @@ -162,41 +162,37 @@ private static async Task WriteByteAsync(FileStream fs, byte value) await fs.FlushAsync(); } - private static List _devicePathsAvailable; - private static List GetDevicePaths() + private static Lazy _availableDevicePaths = new Lazy(() => { - if (_devicePathsAvailable is null) + List paths = new(); + FileStreamOptions options = new { Access = FileAccess.Write, Share = FileShare.Write }; + + foreach (string devicePath in new[] { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }) { - var options = new FileStreamOptions { Access = FileAccess.Write, Share = FileShare.Write }; - _devicePathsAvailable = new List(); + if (!File.Exists(devicePath)) + { + continue; + } - foreach (string devicePath in new[] { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }) + try { - if (!File.Exists(devicePath)) + File.Open(devicePath, options).Dispose(); + } + catch (Exception ex) + { + if (ex is IOException || ex is UnauthorizedAccessException) { continue; } - try - { - File.Open(devicePath, options).Dispose(); - } - catch (Exception ex) - { - if (ex is IOException || ex is UnauthorizedAccessException) - { - continue; - } - - throw; - } - - _devicePathsAvailable.Add(devicePath); + throw; } + + paths.Add(devicePath); } - return _devicePathsAvailable; - } + return paths.ToArray(); + }); public static IEnumerable DevicePath_FileOptions_TestData() { From 7b79ad3e98a6ebabc8aeabcb14cdf6a6be2361e9 Mon Sep 17 00:00:00 2001 From: Jozkee Date: Mon, 11 Oct 2021 10:42:17 -0700 Subject: [PATCH 13/13] Use IEnumerable in Lazy and fix usages --- .../tests/FileStream/DevicesPipesAndSockets.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs index 182d8c9e51c18..ede852c7305b3 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs @@ -162,10 +162,10 @@ private static async Task WriteByteAsync(FileStream fs, byte value) await fs.FlushAsync(); } - private static Lazy _availableDevicePaths = new Lazy(() => + private static Lazy> AvailableDevicePaths = new Lazy>(() => { List paths = new(); - FileStreamOptions options = new { Access = FileAccess.Write, Share = FileShare.Write }; + FileStreamOptions options = new() { Access = FileAccess.Write, Share = FileShare.Write }; foreach (string devicePath in new[] { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" }) { @@ -191,12 +191,12 @@ private static async Task WriteByteAsync(FileStream fs, byte value) paths.Add(devicePath); } - return paths.ToArray(); + return paths; }); public static IEnumerable DevicePath_FileOptions_TestData() { - foreach (string devicePath in GetDevicePaths()) + foreach (string devicePath in AvailableDevicePaths.Value) { foreach (FileOptions options in new[] { FileOptions.None, FileOptions.Asynchronous }) { @@ -207,7 +207,7 @@ public static IEnumerable DevicePath_FileOptions_TestData() public static IEnumerable DevicePath_TestData() { - foreach (string devicePath in GetDevicePaths()) + foreach (string devicePath in AvailableDevicePaths.Value) { yield return new object[] { devicePath }; }