Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
* Cache whether a handle supports random access * Keep avoiding pread…
…/pwrite for unseekable files * Fallback to read/write only for known error ENXIO
  • Loading branch information
David Cantu authored and github-actions committed Oct 12, 2021
commit 3c9804edde193ecfc09cf1b8491b76191ff91fe9
Original file line number Diff line number Diff line change
Expand Up @@ -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 */ }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,27 @@ 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 = 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);
}

Expand Down Expand Up @@ -97,12 +112,27 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<by
// 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 bytesToWrite = GetNumberOfBytesToWrite(buffer.Length);
int bytesWritten = Interop.Sys.PWrite(handle, bufPtr, bytesToWrite, fileOffset);
int bytesWritten;
if (handle.SupportsRandomAccess)
{
bytesWritten = Interop.Sys.PWrite(handle, bufPtr, bytesToWrite, fileOffset);
if (bytesWritten == -1)
{
// Some devices (such as /dev/tty) are reported as seekable by macOS but pwrite is unable to handle them and returns ENXIO.
// Historically we were able to handle /dev/tty using write so we need to fallback to write for that case.
Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
// We want to discover more errors that could make pwrite fail and add unit tests for them.
Debug.Assert(errorInfo.Error == Interop.Error.ENXIO);

// Fallback to write if pwrite fails.
if (bytesWritten == -1)
if (errorInfo.Error == Interop.Error.ENXIO)
{
handle.SupportsRandomAccess = false;
bytesWritten = Interop.Sys.Write(handle, bufPtr, bytesToWrite);
}
}
}
else
{
Debug.Assert(Interop.Sys.GetLastErrorInfo().Error == Interop.Error.ENXIO || !handle.CanSeek); // We want to catch other errors and add unit tests for them.
bytesWritten = Interop.Sys.Write(handle, bufPtr, bytesToWrite);
}

Expand Down