Skip to content

Commit

Permalink
Fallback to read/write if pread/pwrite fails (#59846)
Browse files Browse the repository at this point in the history
* Fallback to read/write if pread/pwrite fails

* * Cache whether a handle supports random access
* Keep avoiding pread/pwrite for unseekable files
* Fallback to read/write only for known error ENXIO

* Test more device paths and add tests for WriteAll*

* Fix CI: skip test when device can't be opened

* * Do not use SkipTestException
* Exclude EBADF from assertion

* use NullableBool in _supportsRandomAccess

* Add tests for mkfifo and socketpair

* Rename test file

* Address feedback in src

* Address suggestions

* Address suggestions

* Update src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs

Use Lazy for thread-safety

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>

* Use IEnumerable<string> in Lazy and fix usages

Co-authored-by: David Cantu <jozkee@macbook.lan>
Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
  • Loading branch information
3 people committed Oct 11, 2021
1 parent 937d451 commit 825b264
Show file tree
Hide file tree
Showing 4 changed files with 287 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
// 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 Xunit;

namespace System.IO.Tests
{
[PlatformSpecific(TestPlatforms.AnyUnix)]
public class DevicesPipesAndSockets : FileSystemTest
{
[Theory]
[MemberData(nameof(DevicePath_FileOptions_TestData))]
public void CharacterDevice_FileStream_Write(string devicePath, FileOptions fileOptions)
{
FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write, Share = FileShare.Write };
using FileStream fs = new(devicePath, options);
fs.Write(Encoding.UTF8.GetBytes("foo"));
}

[Theory]
[MemberData(nameof(DevicePath_FileOptions_TestData))]
public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileOptions fileOptions)
{
FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write, Share = FileShare.Write };
using FileStream fs = new(devicePath, options);
await fs.WriteAsync(Encoding.UTF8.GetBytes("foo"));
}

[Theory]
[MemberData(nameof(DevicePath_TestData))]
public void CharacterDevice_WriteAllBytes(string devicePath)
{
File.WriteAllBytes(devicePath, Encoding.UTF8.GetBytes("foo"));
}

[Theory]
[MemberData(nameof(DevicePath_TestData))]
public async Task CharacterDevice_WriteAllBytesAsync(string devicePath)
{
await File.WriteAllBytesAsync(devicePath, Encoding.UTF8.GetBytes("foo"));
}

[Theory]
[MemberData(nameof(DevicePath_TestData))]
public void CharacterDevice_WriteAllText(string devicePath)
{
File.WriteAllText(devicePath, "foo");
}

[Theory]
[MemberData(nameof(DevicePath_TestData))]
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 = new FileStream(fifoPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
ReadByte(fs, 42);
}),
Task.Run(() =>
{
using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read);
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 = new FileStream(fifoPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
await ReadByteAsync(fs, 42);
}),
Task.Run(async () =>
{
using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read);
await WriteByteAsync(fs, 42);
}));
}

private const int AF_UNIX = 1;
private const int SOCK_STREAM = 1;

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
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: 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();
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
public void SocketPair_ReadWrite_Async()
{
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: 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();
}
}

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 Lazy<IEnumerable<string>> AvailableDevicePaths = new Lazy<IEnumerable<string>>(() =>
{
List<string> paths = new();
FileStreamOptions options = new() { Access = FileAccess.Write, Share = FileShare.Write };
foreach (string devicePath in new[] { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" })
{
if (!File.Exists(devicePath))
{
continue;
}
try
{
File.Open(devicePath, options).Dispose();
}
catch (Exception ex)
{
if (ex is IOException || ex is UnauthorizedAccessException)
{
continue;
}
throw;
}
paths.Add(devicePath);
}
return paths;
});

public static IEnumerable<object[]> DevicePath_FileOptions_TestData()
{
foreach (string devicePath in AvailableDevicePaths.Value)
{
foreach (FileOptions options in new[] { FileOptions.None, FileOptions.Asynchronous })
{
yield return new object[] { devicePath, options};
}
}
}

public static IEnumerable<object[]> DevicePath_TestData()
{
foreach (string devicePath in AvailableDevicePaths.Value)
{
yield return new object[] { devicePath };
}
}

[DllImport("libc")]
private static unsafe extern int socketpair(int domain, int type, int protocol, int* ptr);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
<Compile Include="FileInfo\GetSetAttributes.cs" />
<Compile Include="FileInfo\Length.cs" />
<Compile Include="FileInfo\Open.cs" />
<Compile Include="FileStream\DevicesPipesAndSockets.cs" />
<Compile Include="FileStream\FlushAsync.cs" />
<Compile Include="FileStream\FileStreamConformanceTests.cs" />
<Compile Include="FileStream\SafeFileHandle.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -33,6 +34,25 @@ private SafeFileHandle(bool ownsHandle)

internal bool CanSeek => !IsClosed && GetCanSeek();

internal bool SupportsRandomAccess
{
get
{
NullableBool supportsRandomAccess = _supportsRandomAccess;
if (supportsRandomAccess == NullableBool.Undefined)
{
_supportsRandomAccess = supportsRandomAccess = GetCanSeek() ? NullableBool.True : NullableBool.False;
}

return supportsRandomAccess == NullableBool.True;
}
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;

internal void EnsureThreadPoolBindingInitialized() { /* nop */ }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,30 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span<byte> 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;
if (handle.SupportsRandomAccess)
{
// Try pread for seekable files.
result = Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset);
if (result == -1)
{
// 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();

if (errorInfo.Error == Interop.Error.ENXIO ||
errorInfo.Error == Interop.Error.ESPIPE)
{
handle.SupportsRandomAccess = false;
result = Interop.Sys.Read(handle, bufPtr, buffer.Length);
}
}
}
else
{
result = Interop.Sys.Read(handle, bufPtr, buffer.Length);
}

FileStreamHelpers.CheckFileCall(result, handle.Path);
return result;
}
Expand Down Expand Up @@ -90,9 +111,29 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<by
// The Windows implementation uses WriteFile, which ignores the offset if the handle
// isn't seekable. We do the same manually with PWrite vs Write, in order to enable
// the function to be used by FileStream for all the same situations.
int bytesWritten = handle.CanSeek ?
Interop.Sys.PWrite(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length), fileOffset) :
Interop.Sys.Write(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length));
int bytesToWrite = GetNumberOfBytesToWrite(buffer.Length);
int bytesWritten;
if (handle.SupportsRandomAccess)
{
bytesWritten = Interop.Sys.PWrite(handle, bufPtr, bytesToWrite, fileOffset);
if (bytesWritten == -1)
{
// 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();

if (errorInfo.Error == Interop.Error.ENXIO ||
errorInfo.Error == Interop.Error.ESPIPE)
{
handle.SupportsRandomAccess = false;
bytesWritten = Interop.Sys.Write(handle, bufPtr, bytesToWrite);
}
}
}
else
{
bytesWritten = Interop.Sys.Write(handle, bufPtr, bytesToWrite);
}

FileStreamHelpers.CheckFileCall(bytesWritten, handle.Path);
if (bytesWritten == buffer.Length)
Expand Down

0 comments on commit 825b264

Please sign in to comment.