Fix potential spin in interop message relay (#41319)
The original logic ignores the success result of the initial ReadFile call. If the connection is a socket and its close is not captured by GetOverlappedResult. It could cause this ReadFile call to return success with bytes read == 0. This will put this loop into a spin. This PR refactors the loop with io::MultiHandleWait.
Feng Wang committed
Aug 18, 2026 at 11:10 UTC
725954f882a2eeea60b7a730b92a3ee2e9e3acd9
1 file changed
+56
-58
src/windows/common/interop.cpp
+56
-58
@@ -14,6 +14,7 @@ Abstract:
14
15
#include "precomp.h"
16
#include "interop.hpp"
17
+#include "HandleIO.h"
18
#include "helpers.hpp"
19
#include "socket.hpp"
20
#include "hvsocket.hpp"
@@ -408,78 +409,75 @@ std::string FormatCommandLine(gsl::span<gsl::byte> CommandLineData, USHORT Comma
409
DWORD
410
ProcessInteropMessages(_In_ HANDLE MessageHandle, _Inout_ CreateProcessResult* Result)
411
{
411
- OVERLAPPED Overlapped = {0};
412
- const wil::unique_event OverlappedEvent(wil::EventOptions::ManualReset);
413
- Overlapped.hEvent = OverlappedEvent.get();
414
- const HANDLE WaitHandles[] = {Overlapped.hEvent, Result->Process.get()};
412
+ namespace io = wsl::windows::common::io;
413
416
- // Read messages from the message handle. Break out of the loop if the pipe
417
- // is connection is closed or the process exits.
418
- //
419
- // N.B. ReadFile will automatically reset the event in the overlapped
420
- // structure.
421
- DWORD ExitCode = 1;
422
- for (;;)
423
- {
424
- DWORD BytesRead;
425
- LX_INIT_WINDOW_SIZE_CHANGED WindowSizeMessage;
426
- bool Success = ReadFile(MessageHandle, &WindowSizeMessage, sizeof(WindowSizeMessage), &BytesRead, &Overlapped);
427
- if (!Success)
428
- {
429
- const auto LastError = GetLastError();
430
- if ((LastError == ERROR_BROKEN_PIPE) || (LastError == ERROR_HANDLE_EOF))
431
- {
432
- if (WI_IsFlagClear(Result->Flags, LX_INIT_CREATE_PROCESS_RESULT_FLAG_GUI_APPLICATION))
433
- {
434
- THROW_IF_WIN32_BOOL_FALSE(TerminateProcess(Result->Process.get(), 1));
435
- }
414
+ DWORD exitCode = 1;
415
+ std::vector<char> pending;
416
437
- break;
438
- }
417
+ static_assert(sizeof(LX_INIT_WINDOW_SIZE_CHANGED) % alignof(LX_INIT_WINDOW_SIZE_CHANGED) == 0);
418
440
- THROW_LAST_ERROR_IF(LastError != ERROR_IO_PENDING);
419
+ auto processExit = [&] {
420
+ THROW_IF_WIN32_BOOL_FALSE(GetExitCodeProcess(Result->Process.get(), &exitCode));
421
442
- auto CancelIo = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&] {
443
- CancelIoEx(MessageHandle, &Overlapped);
444
- GetOverlappedResult(MessageHandle, &Overlapped, &BytesRead, TRUE);
445
- });
422
+ // Close the pseudoconsole, this causes all pending data to be flushed.
423
+ Result->PseudoConsole.reset();
424
+ };
425
447
- const DWORD WaitStatus = WaitForMultipleObjects(RTL_NUMBER_OF(WaitHandles), WaitHandles, FALSE, INFINITE);
448
- if (WaitStatus == WAIT_OBJECT_0)
449
- {
450
- Success = GetOverlappedResult(MessageHandle, &Overlapped, &BytesRead, FALSE);
451
- CancelIo.release();
452
- if ((!Success) || (BytesRead == 0))
426
+ io::MultiHandleWait wait;
427
+ wait.AddHandle(
428
+ std::make_unique<io::ReadHandle>(
429
+ io::HandleWrapper{MessageHandle},
430
+ [&](const gsl::span<char>& input) {
431
+ if (input.empty())
432
{
454
- if (WI_IsFlagClear(Result->Flags, LX_INIT_CREATE_PROCESS_RESULT_FLAG_GUI_APPLICATION))
433
+ const DWORD waitStatus = WaitForSingleObject(Result->Process.get(), 0);
434
+ if (waitStatus == WAIT_OBJECT_0)
435
+ {
436
+ processExit();
437
+ }
438
+ else
439
{
456
- THROW_IF_WIN32_BOOL_FALSE(TerminateProcess(Result->Process.get(), 1));
440
+ THROW_HR_IF(E_UNEXPECTED, waitStatus != WAIT_TIMEOUT);
441
+ if (WI_IsFlagClear(Result->Flags, LX_INIT_CREATE_PROCESS_RESULT_FLAG_GUI_APPLICATION))
442
+ {
443
+ THROW_IF_WIN32_BOOL_FALSE(TerminateProcess(Result->Process.get(), 1));
444
+ }
445
}
446
459
- break;
447
+ return;
448
}
449
462
- WI_ASSERT((BytesRead == sizeof(WindowSizeMessage)) && (WindowSizeMessage.Header.MessageType == LxInitMessageWindowSizeChanged));
450
+ std::vector<char> stitchedInput;
451
+ auto remaining = input;
452
+ if (!pending.empty())
453
+ {
454
+ stitchedInput.reserve(pending.size() + input.size());
455
+ stitchedInput.insert(stitchedInput.end(), pending.begin(), pending.end());
456
+ stitchedInput.insert(stitchedInput.end(), input.begin(), input.end());
457
+ pending.clear();
458
+ remaining = gsl::make_span(stitchedInput);
459
+ }
460
464
- const COORD Size{static_cast<SHORT>(WindowSizeMessage.Columns), static_cast<SHORT>(WindowSizeMessage.Rows)};
465
- THROW_IF_FAILED(ResizePseudoConsole(Result->PseudoConsole.get(), Size));
466
- }
467
- else if (WaitStatus == (WAIT_OBJECT_0 + 1))
468
- {
469
- THROW_IF_WIN32_BOOL_FALSE(GetExitCodeProcess(Result->Process.get(), &ExitCode));
461
+ while (remaining.size() >= sizeof(LX_INIT_WINDOW_SIZE_CHANGED))
462
+ {
463
+ const auto* message = gslhelpers::get_struct<const LX_INIT_WINDOW_SIZE_CHANGED>(remaining);
464
+ THROW_HR_IF(
465
+ E_UNEXPECTED,
466
+ (message->Header.MessageType != LxInitMessageWindowSizeChanged) || (message->Header.MessageSize != sizeof(*message)));
467
+
468
+ const COORD size{static_cast<SHORT>(message->Columns), static_cast<SHORT>(message->Rows)};
469
+ THROW_IF_FAILED(ResizePseudoConsole(Result->PseudoConsole.get(), size));
470
+ remaining = remaining.subspan(sizeof(*message));
471
+ }
472
471
- // Close the pseudoconsole, this causes all pending data to be flushed.
472
- Result->PseudoConsole.reset();
473
- break;
474
- }
475
- else
476
- {
477
- THROW_HR(E_UNEXPECTED);
478
- }
479
- }
480
- }
473
+ pending.assign(remaining.begin(), remaining.end());
474
+ }),
475
+ io::MultiHandleWait::CancelOnCompleted);
476
+
477
+ wait.AddHandle(std::make_unique<io::EventHandle>(io::HandleWrapper{Result->Process.get()}, processExit), io::MultiHandleWait::CancelOnCompleted);
478
482
- return ExitCode;
479
+ wait.Run(std::nullopt);
480
+ return exitCode;
481
}
482
483
} // namespace