Fix potential spin in interop message relay

[Feng Wang (chemwolf6922)]

Summary of the Pull Request

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.

PR Checklist

• Closes: ProcessInteropMessages spins at 100% CPU forever when an interop control socket reaches EOF #41173
• Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• Tests: Added/updated if needed and all pass
• Localization: All end user facing strings can be localized
• Dev docs: Added/updated if needed
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

This issue is triggered by a race condition. I'm not able to repro the original issue on my machine.
The normal function was tested locally.

[Copilot]

Pull request overview

This PR fixes an infinite-spin condition in WSL interop’s control-message relay loop (ProcessInteropMessages) by handling the synchronous ReadFile success case where BytesRead == 0 (EOF). This aligns the synchronous path with the existing overlapped-completion path and prevents runaway CPU usage when the interop control socket reaches EOF.

Changes:

• Initialize BytesRead and add a unified EOF/failed-read exit check ((!Success) || (BytesRead == 0)) that applies to both synchronous and asynchronous completion paths.
• Refactor the ERROR_IO_PENDING handling block so overlapped completion updates Success/BytesRead, then uses the shared exit/termination logic.
• Preserve existing behavior for process-exit handling (retrieving ExitCode, resetting pseudoconsole, and breaking).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Blue (OneBlue)]

Change looks good, minor comments. Feel free to address in a followup

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/windows/common/interop.cpp:418

• MultiHandleWait can deliver the process handle completion and the message-handle read completion in either order. Since processExit() resets Result->PseudoConsole, a subsequent read callback in the same Run() iteration can still attempt ResizePseudoConsole(Result->PseudoConsole.get(), ...) and throw due to a null/closed HPCON. Consider guarding the read callback after process exit so late read completions don’t touch the pseudoconsole.

    DWORD exitCode = 1;
    size_t pendingSize = 0;
    LX_INIT_WINDOW_SIZE_CHANGED pendingMessage{};

    auto processExit = [&] {

src/windows/common/interop.cpp:440

• On the EOF path (input.empty()), WaitForSingleObject failure is currently surfaced as E_UNEXPECTED via THROW_HR_IF, which drops GetLastError() context. It’s better to explicitly handle WAIT_FAILED with THROW_LAST_ERROR() so failures are diagnosable.

                    const DWORD waitStatus = WaitForSingleObject(Result->Process.get(), 0);
                    if (waitStatus == WAIT_OBJECT_0)
                    {
                        processExit();
                    }
                    else
                    {
                        THROW_HR_IF(E_UNEXPECTED, waitStatus != WAIT_TIMEOUT);
                        if (WI_IsFlagClear(Result->Flags, LX_INIT_CREATE_PROCESS_RESULT_FLAG_GUI_APPLICATION))

[Feng Wang (chemwolf6922)]

Hi Blue (@OneBlue) . I refactored the implementation with io::MultiHandleWait following your suggestion. Could you help take another look? Thanks.

[Blue (OneBlue)]

I think we can skip the copy into pendingMessage here. If we have enough bytes for a full message, we could just access it via:

const auto& message = gslhelper::get_struct<LX_INIT_WINDOW_SIZE_CHANGED>(remaining); 

[Blue (OneBlue)]

remaining should be stored outside of this lambda, since it's possible for a read to be partial, and if that happens, we want the bytes to be stored until the next call.

In an ideal world, we'd reuse ReadSocketMessageHandle, but it currently only supports sockets. Maybe we can add a flag to add support for non-socket handles, but that's outside the scope of this change