Skip to content

Commit c712ebf

Browse files
Ben HillisCopilot
andcommitted
Fix IOCP double-completion race and check SetFileCompletionNotificationModes
Two fixes: 1. relay.cpp: Call SetFileCompletionNotificationModes BEFORE CreateIoCompletionPort. If FILE_SKIP_COMPLETION_PORT_ON_SUCCESS is not supported for a handle type (e.g. certain socket types), fall back to event-based mode entirely. Previously the handle was associated with the IOCP without skip-on-success, causing synchronous completions to both be processed inline by Schedule() and queued to the IOCP — a double-processing race that manifests as ERROR_NO_DATA (232) in ContainerLogs/ContainerAttach tests. 2. localhost.cpp: THROW_IF_WIN32_BOOL_FALSE on SetFileCompletionNotificationModes in the port relay AcceptThread. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c5e6526 commit c712ebf

File tree

2 files changed

+29
-18
lines changed

2 files changed

+29
-18
lines changed

src/windows/common/relay.cpp

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,14 +1219,20 @@ void ReadHandle::Register(HANDLE iocp, OverlappedIOHandle* completionTarget)
12191219
{
12201220
// Try to associate the handle with the IOCP. Not all handle types support this
12211221
// (e.g., some socket types, console handles). Fall back to event-based mode on failure.
1222-
auto result = CreateIoCompletionPort(Handle.Get(), iocp, reinterpret_cast<ULONG_PTR>(completionTarget), 0);
1223-
if (result != nullptr)
1222+
//
1223+
// N.B. FILE_SKIP_COMPLETION_PORT_ON_SUCCESS is required before associating with the IOCP.
1224+
// Without it, synchronous completions would both be processed inline by Schedule() AND
1225+
// queue an IOCP packet, causing double-processing. If it's not supported for this handle
1226+
// type, fall back to event-based mode entirely.
1227+
if (SetFileCompletionNotificationModes(Handle.Get(), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS))
12241228
{
1225-
SetFileCompletionNotificationModes(Handle.Get(), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS);
1226-
1227-
// Clear the event from OVERLAPPED — completions now go to the IOCP.
1228-
Overlapped.hEvent = nullptr;
1229-
RegisteredWithIocp = true;
1229+
auto result = CreateIoCompletionPort(Handle.Get(), iocp, reinterpret_cast<ULONG_PTR>(completionTarget), 0);
1230+
if (result != nullptr)
1231+
{
1232+
// Clear the event from OVERLAPPED — completions now go to the IOCP.
1233+
Overlapped.hEvent = nullptr;
1234+
RegisteredWithIocp = true;
1235+
}
12301236
}
12311237
// else: fall back to event-based mode (Overlapped.hEvent remains set)
12321238
}
@@ -1362,12 +1368,14 @@ void SingleAcceptHandle::Register(HANDLE iocp, OverlappedIOHandle* completionTar
13621368
Iocp = iocp;
13631369
if (!RegisteredWithIocp)
13641370
{
1365-
auto result = CreateIoCompletionPort(ListenSocket.Get(), iocp, reinterpret_cast<ULONG_PTR>(completionTarget), 0);
1366-
if (result != nullptr)
1371+
if (SetFileCompletionNotificationModes(ListenSocket.Get(), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS))
13671372
{
1368-
SetFileCompletionNotificationModes(ListenSocket.Get(), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS);
1369-
Overlapped.hEvent = nullptr;
1370-
RegisteredWithIocp = true;
1373+
auto result = CreateIoCompletionPort(ListenSocket.Get(), iocp, reinterpret_cast<ULONG_PTR>(completionTarget), 0);
1374+
if (result != nullptr)
1375+
{
1376+
Overlapped.hEvent = nullptr;
1377+
RegisteredWithIocp = true;
1378+
}
13711379
}
13721380
}
13731381
}
@@ -1618,12 +1626,14 @@ void WriteHandle::Register(HANDLE iocp, OverlappedIOHandle* completionTarget)
16181626
Iocp = iocp;
16191627
if (!RegisteredWithIocp)
16201628
{
1621-
auto result = CreateIoCompletionPort(Handle.Get(), iocp, reinterpret_cast<ULONG_PTR>(completionTarget), 0);
1622-
if (result != nullptr)
1629+
if (SetFileCompletionNotificationModes(Handle.Get(), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS))
16231630
{
1624-
SetFileCompletionNotificationModes(Handle.Get(), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS);
1625-
Overlapped.hEvent = nullptr;
1626-
RegisteredWithIocp = true;
1631+
auto result = CreateIoCompletionPort(Handle.Get(), iocp, reinterpret_cast<ULONG_PTR>(completionTarget), 0);
1632+
if (result != nullptr)
1633+
{
1634+
Overlapped.hEvent = nullptr;
1635+
RegisteredWithIocp = true;
1636+
}
16271637
}
16281638
}
16291639
}

src/windows/wslrelay/localhost.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,8 @@ void wsl::windows::wslrelay::localhost::RunWSLCPortRelay(const GUID& VmId, uint3
676676
THROW_LAST_ERROR_IF_NULL(CreateIoCompletionPort(
677677
reinterpret_cast<HANDLE>(e.second->ListenSocket.get()), iocp.get(), reinterpret_cast<ULONG_PTR>(e.second.get()), 0));
678678

679-
SetFileCompletionNotificationModes(reinterpret_cast<HANDLE>(e.second->ListenSocket.get()), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS);
679+
THROW_IF_WIN32_BOOL_FALSE(SetFileCompletionNotificationModes(
680+
reinterpret_cast<HANDLE>(e.second->ListenSocket.get()), FILE_SKIP_COMPLETION_PORT_ON_SUCCESS));
680681

681682
e.second->AssociatedWithIocp = true;
682683
}

0 commit comments

Comments
 (0)