Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fallback to read/write if pread/pwrite fails #59846

Merged
merged 13 commits into from
Oct 11, 2021
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))]
Copy link
Member

Choose a reason for hiding this comment

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

just an opinion: when reviewing this file, the first thing I had to do was to scroll down and find DevicePath_FileOptions_TestData. If it was at the top, I would not need to do that.

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 */ ));
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved

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));
Copy link
Member

Choose a reason for hiding this comment

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

another great idea to test non-seekable files! 👍


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<string[]> _availableDevicePaths = new Lazy<string[]>(() =>
jozkee marked this conversation as resolved.
Show resolved Hide resolved
{
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.ToArray();
});

public static IEnumerable<object[]> DevicePath_FileOptions_TestData()
{
foreach (string devicePath in GetDevicePaths())
{
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 GetDevicePaths())
{
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 @@ -114,6 +114,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;
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
}

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;
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth adding a comment here about benign race conditions. It's possible two threads could interleave in such a way that one thread fails an I/O and sets _supportsRandomAccess to false and then another thread that was in the process of initializing _supportsRandomAccess overwrites it with true based on CanSeek. It's currently benign, however, because it'll get set to false again the next time I/O fails.

}
}

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);
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

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);
}
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved

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