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,253 @@
// 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 };
if (IsDeviceUnreachable(devicePath, options))
{
return;
}

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 };
if (IsDeviceUnreachable(devicePath, options))
{
return;
}

using FileStream fs = new(devicePath, options);
await fs.WriteAsync(Encoding.UTF8.GetBytes("foo"));
}

[Theory]
[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"));
}

[Theory]
[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"));
}

[Theory]
[MemberData(nameof(DevicePath_TestData))]
public void CharacterDevice_WriteAllText(string devicePath)
{
if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Access = FileAccess.Write }))
{
return;
}

File.WriteAllText(devicePath, "foo");
}

[Theory]
[MemberData(nameof(DevicePath_TestData))]
public async Task CharacterDevice_WriteAllTextAsync(string devicePath)
{
if (IsDeviceUnreachable(devicePath, new FileStreamOptions{ Options = FileOptions.Asynchronous, Access = FileAccess.Write }))
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}

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

Choose a reason for hiding this comment

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

these consts are used in more than one method, they could be declared in the type or you could move the socketpair-related logic to a separate method:

private static unsafe (FileStream read, FileStream write) CreateSocketPair()
{
    int* ptr = stackalloc int[2];
    Assert.Equal(0, socketpair(1, 1, 0, ptr));
 
    return (new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: true), FileAccess.Read), 
            new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: true), FileAccess.Write));
}

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

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)
{
jozkee marked this conversation as resolved.
Show resolved Hide resolved
if (!File.Exists(devicePath))
{
return true;
}

try
{
File.Open(devicePath, options).Dispose();
}
catch (Exception ex)
{
if (ex is IOException || ex is UnauthorizedAccessException)
{
return true;
}

throw;
}

return false;
}

private static string[] DevicePaths = { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" };

public static IEnumerable<object[]> DevicePath_FileOptions_TestData()
{
foreach (string devicePath in DevicePaths)
{
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 DevicePaths)
{
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);
}
}
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,20 @@ private SafeFileHandle(bool ownsHandle)

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

internal bool SupportsRandomAccess
{
get
{
if (_supportsRandomAccess == NullableBool.Undefined)
jozkee marked this conversation as resolved.
Show resolved Hide resolved
{
_supportsRandomAccess = GetCanSeek() ? NullableBool.True : NullableBool.False;
}

return _supportsRandomAccess == NullableBool.True;
}
set => _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.

nit: This setter should only be used to set to false. Maybe we should add an Assert?

}

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 ||
jozkee marked this conversation as resolved.
Show resolved Hide resolved
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