Skip to content

Don't use callbacks when performing sync IO with async handles #126845

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.

Sign up for GitHub

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

Open
Copilot wants to merge 19 commits into
base: main
Choose a base branch
from
+189 −105
Open

Don't use callbacks when performing sync IO with async handles #126845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4d6f586
Initial plan
Copilot Apr 13, 2026
b3325a4
Replace CallbackResetEvent with ManualResetEvent and low-order bit hE…
Copilot Apr 13, 2026
a34ad37
Address review nits: use target-typed new and rename AllocNativeOverl…
Copilot Apr 14, 2026
e387e23
Merge branch 'main' into copilot/remove-callbacks-sync-io
adamsitnik Apr 14, 2026
99c34d4
Remove unnecessary InternalLow zeroing since NativeMemory.Free has no…
Copilot Apr 14, 2026
d0fc732
Replace ManualResetEvent + WaitOne with native CreateEventExW + GetOv…
Copilot Apr 14, 2026
8ed0afc
Cache event handle in SafeFileHandle, reuse existing CreateEventEx P/…
Copilot Apr 25, 2026
ac128bb
Move event creation logic into SafeFileHandle.RentSyncWaitHandle
Copilot Apr 25, 2026
38cf5d0
Use cached ManualResetEvent + CancelIoEx for safe sync-over-async IO
Copilot Jun 8, 2026
b9f8d12
Only call WaitOne for ERROR_IO_PENDING, extend test to verify handle …
Copilot Jun 8, 2026
7810dee
fix the new test by calling SetWaitNotificationRequired
adamsitnik Jun 8, 2026
a8e69ef
Handle sync completions and stabilize wait-path test
Copilot Jun 8, 2026
c4d02a6
Adjust pipe recovery read loop offset
Copilot Jun 8, 2026
569ccce
Use stack NativeOverlapped and add SafeHandle AddRef guards
Copilot Jun 9, 2026
9d8f0f8
Revert "Use stack NativeOverlapped and add SafeHandle AddRef guards"
adamsitnik Jun 9, 2026
cfac97c
use DangerousAddRef+DangerousRelease for SafeWaitHandle passed to Nat…
adamsitnik Jun 9, 2026
b713596
address feedback: keep SFH alive for whole overlapped operation
adamsitnik Jun 9, 2026
8d5ec30
address code reviewer feedback
adamsitnik Jun 10, 2026
cb8638f
Merge branch 'main' into copilot/remove-callbacks-sync-io
jkotas Jun 10, 2026
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
28 changes: 28 additions & 0 deletions ...reLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.OverlappedValueTaskSource.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,45 @@ namespace Microsoft.Win32.SafeHandles
public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
{
private OverlappedValueTaskSource? _reusableOverlappedValueTaskSource; // reusable OverlappedValueTaskSource that is currently NOT being used
private ManualResetEvent? _reusableSyncWaitEvent; // reusable event for sync-over-async I/O

// Rent the reusable OverlappedValueTaskSource, or create a new one to use if we couldn't get one (which
// should only happen on first use or if the SafeFileHandle is being used concurrently).
internal OverlappedValueTaskSource GetOverlappedValueTaskSource() =>
Interlocked.Exchange(ref _reusableOverlappedValueTaskSource, null) ?? new OverlappedValueTaskSource(this);

// Rent the reusable ManualResetEvent for sync-over-async I/O, or create a new one.
// The returned event is guaranteed to be in non-signaled state.
internal ManualResetEvent RentSyncWaitEvent()
{
ManualResetEvent? mre = Interlocked.Exchange(ref _reusableSyncWaitEvent, null);
if (mre is not null)
{
mre.Reset();
return mre;
}

return new ManualResetEvent(false);
}

internal void ReturnSyncWaitEvent(ManualResetEvent waitEvent)
{
if (Interlocked.CompareExchange(ref _reusableSyncWaitEvent, waitEvent, null) is not null)
{
waitEvent.Dispose();
}
else if (IsClosed)
{
Interlocked.Exchange(ref _reusableSyncWaitEvent, null)?.Dispose();
}
}

protected override bool ReleaseHandle()
{
bool result = Interop.Kernel32.CloseHandle(handle);

Interlocked.Exchange(ref _reusableOverlappedValueTaskSource, null)?.Dispose();
Interlocked.Exchange(ref _reusableSyncWaitEvent, null)?.Dispose();

return result;
}
Expand Down
190 changes: 85 additions & 105 deletions src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ namespace System.IO
{
public static partial class RandomAccess
{
private static readonly IOCompletionCallback s_callback = AllocateCallback();

internal static unsafe void SetFileLength(SafeFileHandle handle, long length)
{
var eofInfo = new Interop.Kernel32.FILE_END_OF_FILE_INFO
Expand Down Expand Up @@ -67,68 +65,74 @@ _ when IsEndOfFile(errorCode, handle, fileOffset) => 0,
}
}

private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span<byte> buffer, long fileOffset)
private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle fileHandle, Span<byte> buffer, long fileOffset)
{
handle.EnsureThreadPoolBindingInitialized();

CallbackResetEvent resetEvent = new CallbackResetEvent(handle.ThreadPoolBinding!);
NativeOverlapped* overlapped = null;
ManualResetEvent waitEvent = fileHandle.RentSyncWaitEvent();
SafeWaitHandle waitHandle = waitEvent.SafeWaitHandle;
bool releaseWaitHandle = false, releaseFileHandle = false;

try
{
overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, resetEvent);
fileHandle.DangerousAddRef(ref releaseFileHandle); // keep it alive for the whole overlapped operation
waitHandle.DangerousAddRef(ref releaseWaitHandle);
NativeOverlapped overlapped = GetNativeOverlappedForAsyncHandle(fileHandle, fileOffset, waitHandle.DangerousGetHandle());

fixed (byte* pinned = &MemoryMarshal.GetReference(buffer))
{
Interop.Kernel32.ReadFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped);

int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle);
if (errorCode == Interop.Errors.ERROR_IO_PENDING)
int errorCode = Interop.Kernel32.ReadFile(fileHandle, pinned, buffer.Length, IntPtr.Zero, &overlapped) == 0
? FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(fileHandle)
: Interop.Errors.ERROR_SUCCESS;
if (errorCode is Interop.Errors.ERROR_IO_PENDING)
{
resetEvent.WaitOne();
errorCode = Interop.Errors.ERROR_SUCCESS;
try
{
waitEvent.WaitOne();
errorCode = Interop.Errors.ERROR_SUCCESS;
}
catch
{
// WaitOne can throw arbitrary exceptions (e.g., via SynchronizationContext).
// Cancel the pending IO and wait for completion before freeing the overlapped.
Interop.Kernel32.CancelIoEx(fileHandle, &overlapped);
int canceledBytes = 0;
Interop.Kernel32.GetOverlappedResult(fileHandle, &overlapped, ref canceledBytes, bWait: true);
throw;
}
}

if (errorCode == Interop.Errors.ERROR_SUCCESS)
if (errorCode is Interop.Errors.ERROR_SUCCESS)
{
int result = 0;
if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false))
if (Interop.Kernel32.GetOverlappedResult(fileHandle, &overlapped, ref result, bWait: false))
{
Debug.Assert(result >= 0 && result <= buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request");
return result;
}

errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle);
}
else
{
// The initial errorCode was neither ERROR_IO_PENDING nor ERROR_SUCCESS, so the operation
// failed with an error and the callback won't be invoked. We thus need to decrement the
// ref count on the resetEvent that was initialized to a value under the expectation that
// the callback would be invoked and decrement it.
resetEvent.ReleaseRefCount(overlapped);
errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(fileHandle);
}

if (IsEndOfFile(errorCode, handle, fileOffset))
if (IsEndOfFile(errorCode, fileHandle, fileOffset))
{
// EOF on a pipe. Callback will not be called.
// We clear the overlapped status bit for this special case (failure
// to do so looks like we are freeing a pending overlapped later).
overlapped->InternalLow = IntPtr.Zero;
return 0;
}

throw Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path);
throw Win32Marshal.GetExceptionForWin32Error(errorCode, fileHandle.Path);
}
}
finally
{
if (overlapped != null)
if (releaseWaitHandle)
{
resetEvent.ReleaseRefCount(overlapped);
waitHandle.DangerousRelease();
}

resetEvent.Dispose();
if (releaseFileHandle)
{
fileHandle.DangerousRelease();
}
Comment thread
adamsitnik marked this conversation as resolved.

fileHandle.ReturnSyncWaitEvent(waitEvent);
}
}

Expand Down Expand Up @@ -159,51 +163,56 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<by
}
}

private static unsafe void WriteSyncUsingAsyncHandle(SafeFileHandle handle, ReadOnlySpan<byte> buffer, long fileOffset)
private static unsafe void WriteSyncUsingAsyncHandle(SafeFileHandle fileHandle, ReadOnlySpan<byte> buffer, long fileOffset)
{
if (buffer.IsEmpty)
{
return;
}

handle.EnsureThreadPoolBindingInitialized();

CallbackResetEvent resetEvent = new CallbackResetEvent(handle.ThreadPoolBinding!);
NativeOverlapped* overlapped = null;
ManualResetEvent waitEvent = fileHandle.RentSyncWaitEvent();
SafeWaitHandle waitHandle = waitEvent.SafeWaitHandle;
bool releaseWaitHandle = false, releaseFileHandle = false;

try
{
overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, resetEvent);
fileHandle.DangerousAddRef(ref releaseFileHandle); // keep it alive for the whole overlapped operation
waitHandle.DangerousAddRef(ref releaseWaitHandle);
NativeOverlapped overlapped = GetNativeOverlappedForAsyncHandle(fileHandle, fileOffset, waitHandle.DangerousGetHandle());

fixed (byte* pinned = &MemoryMarshal.GetReference(buffer))
{
Interop.Kernel32.WriteFile(handle, pinned, buffer.Length, IntPtr.Zero, overlapped);

int errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle);
if (errorCode == Interop.Errors.ERROR_IO_PENDING)
int errorCode = Interop.Kernel32.WriteFile(fileHandle, pinned, buffer.Length, IntPtr.Zero, &overlapped) == 0
? FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(fileHandle)
: Interop.Errors.ERROR_SUCCESS;
if (errorCode is Interop.Errors.ERROR_IO_PENDING)
{
resetEvent.WaitOne();
errorCode = Interop.Errors.ERROR_SUCCESS;
try
{
waitEvent.WaitOne();
errorCode = Interop.Errors.ERROR_SUCCESS;
}
catch
{
// WaitOne can throw arbitrary exceptions (e.g., via SynchronizationContext).
// Cancel the pending IO and wait for completion before freeing the overlapped.
Interop.Kernel32.CancelIoEx(fileHandle, &overlapped);
int canceledBytes = 0;
Interop.Kernel32.GetOverlappedResult(fileHandle, &overlapped, ref canceledBytes, bWait: true);
throw;
}
}

if (errorCode == Interop.Errors.ERROR_SUCCESS)
if (errorCode is Interop.Errors.ERROR_SUCCESS)
{
int result = 0;
if (Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref result, bWait: false))
if (Interop.Kernel32.GetOverlappedResult(fileHandle, &overlapped, ref result, bWait: false))
{
Debug.Assert(result == buffer.Length, $"GetOverlappedResult returned {result} for {buffer.Length} bytes request");
return;
}

errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(handle);
}
else
{
// The initial errorCode was neither ERROR_IO_PENDING nor ERROR_SUCCESS, so the operation
// failed with an error and the callback won't be invoked. We thus need to decrement the
// ref count on the resetEvent that was initialized to a value under the expectation that
// the callback would be invoked and decrement it.
resetEvent.ReleaseRefCount(overlapped);
errorCode = FileStreamHelpers.GetLastWin32ErrorAndDisposeHandleIfInvalid(fileHandle);
}

throw errorCode switch
Expand All @@ -213,18 +222,23 @@ private static unsafe void WriteSyncUsingAsyncHandle(SafeFileHandle handle, Read
// to a handle opened asynchronously.
Interop.Errors.ERROR_INVALID_PARAMETER => new IOException(SR.IO_FileTooLong),

_ => Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path),
_ => Win32Marshal.GetExceptionForWin32Error(errorCode, fileHandle.Path),
};
}
}
finally
{
if (overlapped != null)
if (releaseWaitHandle)
{
waitHandle.DangerousRelease();
}
Comment thread
adamsitnik marked this conversation as resolved.

if (releaseFileHandle)
{
resetEvent.ReleaseRefCount(overlapped);
fileHandle.DangerousRelease();
}

resetEvent.Dispose();
fileHandle.ReturnSyncWaitEvent(waitEvent);
}
}

Expand Down Expand Up @@ -724,16 +738,15 @@ private static async ValueTask WriteGatherAtOffsetMultipleSyscallsAsync(SafeFile
}
}

private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(SafeFileHandle handle, long fileOffset, CallbackResetEvent resetEvent)
private static NativeOverlapped GetNativeOverlappedForAsyncHandle(SafeFileHandle handle, long fileOffset, nint waitHandle)
{
// After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding
// to allocate a native overlapped and provide a valid callback.
NativeOverlapped* result = handle.ThreadPoolBinding!.UnsafeAllocateNativeOverlapped(s_callback, resetEvent, null);
Debug.Assert(handle.IsAsync);

NativeOverlapped result = default;
if (handle.CanSeek)
{
result->OffsetLow = unchecked((int)fileOffset);
result->OffsetHigh = (int)(fileOffset >> 32);
result.OffsetLow = unchecked((int)fileOffset);
result.OffsetHigh = (int)(fileOffset >> 32);
}

// From https://learn.microsoft.com/windows/win32/api/ioapiset/nf-ioapiset-getoverlappedresult:
Expand All @@ -743,7 +756,11 @@ private static async ValueTask WriteGatherAtOffsetMultipleSyscallsAsync(SafeFile
// are performed on the same file, named pipe, or communications device.
// In this situation, there is no way to know which operation caused the object's state to be signaled."
// Since we want RandomAccess APIs to be thread-safe, we provide a dedicated wait handle.
result->EventHandle = resetEvent.SafeWaitHandle.DangerousGetHandle();
// From https://learn.microsoft.com/windows/win32/api/ioapiset/nf-ioapiset-getqueuedcompletionstatus:
// "If the file handle associated with the completion packet was previously associated with an I/O completion port
// [...] setting the low-order bit of hEvent in the OVERLAPPED structure prevents the I/O completion
// from being queued to a completion port."
result.EventHandle = waitHandle | 1;

return result;
}
Expand All @@ -761,17 +778,6 @@ private static NativeOverlapped GetNativeOverlappedForSyncHandle(SafeFileHandle
return result;
}

private static unsafe IOCompletionCallback AllocateCallback()
{
return new IOCompletionCallback(Callback);

static void Callback(uint errorCode, uint numBytes, NativeOverlapped* pOverlapped)
{
CallbackResetEvent state = (CallbackResetEvent)ThreadPoolBoundHandle.GetNativeOverlappedState(pOverlapped)!;
state.ReleaseRefCount(pOverlapped);
}
}

internal static bool IsEndOfFile(int errorCode, SafeFileHandle handle, long fileOffset)
{
switch (errorCode)
Expand All @@ -798,32 +804,6 @@ internal static bool IsEndOfFile(int errorCode, SafeFileHandle handle, long file
private static bool IsEndOfFileForNoBuffering(SafeFileHandle fileHandle, long fileOffset)
=> fileHandle.IsNoBuffering && fileHandle.CanSeek && fileOffset >= fileHandle.GetFileLength();

// We need to store the reference count (see the comment in ReleaseRefCount) and an EventHandle to signal the completion.
// We could keep these two things separate, but since ManualResetEvent is sealed and we want to avoid any extra allocations, this type has been created.
// It's basically ManualResetEvent with reference count.
private sealed class CallbackResetEvent : EventWaitHandle
{
private readonly ThreadPoolBoundHandle _threadPoolBoundHandle;
private int _freeWhenZero = 2; // one for the callback and another for the method that calls GetOverlappedResult

internal CallbackResetEvent(ThreadPoolBoundHandle threadPoolBoundHandle) : base(initialState: false, EventResetMode.ManualReset)
{
_threadPoolBoundHandle = threadPoolBoundHandle;
}

internal unsafe void ReleaseRefCount(NativeOverlapped* pOverlapped)
{
// Each SafeFileHandle opened for async IO is bound to ThreadPool.
// It requires us to provide a callback even if we want to use EventHandle and use GetOverlappedResult to obtain the result.
// There can be a race condition between the call to GetOverlappedResult and the callback invocation,
// so we need to track the number of references, and when it drops to zero, then free the native overlapped.
if (Interlocked.Decrement(ref _freeWhenZero) == 0)
{
_threadPoolBoundHandle.FreeNativeOverlapped(pOverlapped);
}
}
}

// Abstracts away the type signature incompatibility between Memory and ReadOnlyMemory.
private interface IMemoryHandler<T>
{
Expand Down
Loading
Loading