Fix relay loops spinning at 100% CPU on synchronous EOF#40725
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a high-CPU spin in WSL relay loops by treating synchronous ReadFile success with bytesRead == 0 as EOF, ensuring relay threads terminate instead of repeatedly re-issuing reads after a graceful peer close.
Changes:
- Add explicit
bytesRead == 0EOF handling on synchronous-successReadFilepaths inBidirectionalRelay. - Add explicit zero-byte EOF handling in
ScopedMultiRelay::Runfor both synchronous and overlapped-completion paths. - Add a new
RelayEofDetectionunit test to validate relay termination on EOF scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/windows/common/relay.cpp |
Adds zero-byte read detection to prevent CPU-bound spinning on synchronous EOF in relay loops. |
test/windows/UnitTests.cpp |
Introduces a unit test intended to ensure relay functions terminate promptly on EOF. |
7a41534 to
c06f4bc
Compare
OneBlue
left a comment
There was a problem hiding this comment.
LGTM. I think at some point we should move those to use RelayHandle and factor all the IO scheduling in one place, but for now let's merge this
yep I considered doing that as part of this change, but we can do it as a follow-up. |
c06f4bc to
af6a873
Compare
| // Wait up to 5 seconds for the relay to finish. If it doesn't, the EOF check is broken. | ||
| auto threadHandle = wil::unique_handle(OpenThread(SYNCHRONIZE, FALSE, GetThreadId(relayThread.native_handle()))); | ||
| VERIFY_ARE_NOT_EQUAL(WaitForSingleObject(relayThread.native_handle(), 5000), WAIT_TIMEOUT); | ||
| relayThread.join(); |
| readHandle.reset(CreateNamedPipeW( | ||
| pipeName.c_str(), PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 4096, 4096, 0, &sa)); | ||
| VERIFY_IS_NOT_NULL(readHandle.get()); | ||
|
|
| writeHandle.reset( | ||
| CreateFileW(pipeName.c_str(), GENERIC_WRITE, 0, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)); | ||
| VERIFY_IS_NOT_NULL(writeHandle.get()); |
| VERIFY_ARE_NOT_EQUAL(WaitForSingleObject(relayThread.native_handle(), 5000), WAIT_TIMEOUT); | ||
| relayThread.join(); |
af6a873 to
a4a502a
Compare
a4a502a to
0cfd705
Compare
| // Helper: create an overlapped pipe pair (named pipe server + client). | ||
| auto createOverlappedPipe = [](wil::unique_handle& readHandle, wil::unique_handle& writeHandle) { | ||
| static std::atomic<int> pipeCounter{0}; | ||
| auto pipeName = std::format(L"\\\\.\\pipe\\WslTest_RelayEof_{}", pipeCounter++); | ||
|
|
||
| SECURITY_ATTRIBUTES sa{sizeof(sa), nullptr, TRUE}; | ||
| readHandle.reset(CreateNamedPipeW( | ||
| pipeName.c_str(), PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 4096, 4096, 0, &sa)); | ||
| VERIFY_IS_NOT_NULL(readHandle.get()); | ||
|
|
||
| writeHandle.reset(CreateFileW(pipeName.c_str(), GENERIC_WRITE, 0, &sa, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, nullptr)); | ||
| VERIFY_IS_NOT_NULL(writeHandle.get()); | ||
| }; |
| wil::unique_handle readPipe, writePipe; | ||
| createOverlappedPipe(readPipe, writePipe); | ||
|
|
| // Create an output pipe to capture relayed data. | ||
| wil::unique_handle outputRead, outputWrite; | ||
| createOverlappedPipe(outputRead, outputWrite); | ||
|
|
| // Wait up to 5 seconds for the relay to finish. If it doesn't, the EOF check is broken. | ||
| VERIFY_ARE_NOT_EQUAL(WaitForSingleObject(relayThread.native_handle(), 5000), WAIT_TIMEOUT); | ||
| relayThread.join(); |
| wil::unique_handle leftRead, leftWrite, rightRead, rightWrite; | ||
| createOverlappedPipe(leftRead, leftWrite); | ||
| createOverlappedPipe(rightRead, rightWrite); | ||
|
|
|
|
||
| VERIFY_ARE_NOT_EQUAL(WaitForSingleObject(relayThread.native_handle(), 5000), WAIT_TIMEOUT); | ||
| relayThread.join(); |
| wil::unique_handle read1, write1, read2, write2; | ||
| createOverlappedPipe(read1, write1); | ||
| createOverlappedPipe(read2, write2); | ||
|
|
0cfd705 to
d89dffc
Compare
When ReadFile completes synchronously with 0 bytes read (graceful close on sockets/pipes), the relay loops failed to detect this as EOF and re-issued the read in a tight loop, burning CPU indefinitely. Fix by checking for zero bytes on the synchronous-success path in: - BidirectionalRelay (both left and right sides) - ScopedMultiRelay::Run (both sync and overlapped completion paths) Fixes #40651 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d89dffc to
820c01f
Compare
| readHandle.reset(CreateNamedPipeW( | ||
| pipeName.c_str(), PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 4096, 4096, 0, &sa)); | ||
| VERIFY_IS_NOT_NULL(readHandle.get()); |
| writeHandle.reset(CreateFileW(pipeName.c_str(), GENERIC_WRITE, 0, &sa, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, nullptr)); | ||
| VERIFY_IS_NOT_NULL(writeHandle.get()); |
| serverHandle.reset(CreateNamedPipeW( | ||
| pipeName.c_str(), PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED, PIPE_TYPE_BYTE | PIPE_WAIT, 1, 4096, 4096, 0, &sa)); | ||
| VERIFY_IS_NOT_NULL(serverHandle.get()); |
| clientHandle.reset(CreateFileW(pipeName.c_str(), GENERIC_READ | GENERIC_WRITE, 0, &sa, OPEN_EXISTING, FILE_FLAG_OVERLAPPED, nullptr)); | ||
| VERIFY_IS_NOT_NULL(clientHandle.get()); |
| std::thread([&]() { wsl::windows::common::relay::InterruptableRelay(readPipe.get(), outputWrite.get()); }); | ||
|
|
||
| // Wait up to 5 seconds for the relay to finish. If it doesn't, the EOF check is broken. | ||
| VERIFY_ARE_EQUAL(WaitForSingleObject(relayThread.native_handle(), 5000), WAIT_OBJECT_0); | ||
| relayThread.join(); |
| outputWrite.reset(); | ||
| char buf[64]{}; | ||
| DWORD bytesRead{}; | ||
| ReadFile(outputRead.get(), buf, sizeof(buf), &bytesRead, nullptr); | ||
| VERIFY_ARE_EQUAL(bytesRead, static_cast<DWORD>(testData.size())); | ||
| VERIFY_ARE_EQUAL(std::string_view(buf, bytesRead), testData); |
| auto relayThread = | ||
| std::thread([&]() { wsl::windows::common::relay::BidirectionalRelay(leftServer.get(), rightServer.get()); }); | ||
|
|
||
| VERIFY_ARE_EQUAL(WaitForSingleObject(relayThread.native_handle(), 5000), WAIT_OBJECT_0); | ||
| relayThread.join(); |
| // Sync should return promptly once both inputs hit EOF. | ||
| relay.Sync(); | ||
| } |
Summary
Fixes wsl.exe relay threads spinning at 100% CPU after the peer (VM-side hvsocket) closes gracefully.
Problem
When
ReadFilecompletes synchronously with 0 bytes (the canonical EOF signal on stream sockets and pipes), three relay code paths failed to recognize this as EOF:BidirectionalRelay— used bywslrelay.exefor localhost forwarding and socket relayScopedMultiRelay::Run— used for set-version stderr relayInstead of breaking out of the loop, they re-issued
ReadFileon the EOF'd handle, which returned TRUE + 0 bytes again immediately, producing a pure CPU-bound spin.Fix
Add
bytesRead == 0checks on the synchronous-success path in:BidirectionalRelay(both left and right read sites)ScopedMultiRelay::Run(both the sync-completion and overlapped-completion paths)Validation
Added
RelayEofDetectionunit test that creates pipe pairs, closes the write end (producing synchronous EOF), and verifies each relay function terminates within 5 seconds rather than spinning.Fixes #40651