Fix concurrent console state restoration

[shuaiyuanxx]

Summary of the Pull Request

Fixes a race where concurrent wsl.exe clients sharing one console can restore console modes and code pages out of order, leaving the console in WSL's temporary interactive state.

PR Checklist

• Closes: Fixes Concurrent wsl.exe clients can restore CONIN$ mode out of order and leave VT input enabled #41201
• 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: No end-user-facing strings were added
• Dev docs: No development documentation changes are needed
• Documentation updated: No user documentation changes are needed

Detailed Description of the Pull Request / Additional comments

Each ConsoleState instance saves the current console state before configuring interactive mode. With overlapping clients, a later client can save the temporary mode set by an earlier client. If the clients exit out of order, the later client can restore that stale temporary state.

This change records the effective mode and code page configured by each client. During teardown, saved state is restored only when the current console state still matches that client's configured value. This prevents stale restores and avoids overwriting changes made by another client or external process.

Regression coverage includes overlapping in-process clients, shared-console wsl.exe processes, external mode changes, separate-console isolation, and process termination.

Validation Steps Performed

• Configured the repository with CMake using Visual Studio 18 2026, x64, Debug
• Successfully built the wsltests and wsl targets
• Ran the focused ConsoleState_* TAEF tests
• Result: 5 passed, 0 failed, 0 skipped

[Copilot]

Pull request overview

This PR fixes a race in wsl.exe console-state restoration when multiple WSL clients share the same console: a later client can snapshot another client’s temporary console modes/code pages and restore that stale state after the earlier client exits. The change records the effective modes/code pages applied by each client and restores saved values only if the console still matches what that client configured.

Changes:

• Track per-instance “configured” console input/output modes and code pages in ConsoleState, and gate restoration on whether the console still matches those configured values.
• Add unit and process-level tests covering overlapping console clients, external console mode changes, separate consoles, shared-console process overlap, and termination scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/windows/common/ConsoleState.h	Adds fields to record the effective modes/code pages configured by each ConsoleState instance.
src/windows/common/ConsoleState.cpp	Captures effective configured modes/code pages and conditionally restores only when current console state still matches what this instance set.
test/windows/wslc/WSLCCLIVTSupportUnitTests.cpp	Adds regression coverage for concurrent clients (in-proc and out-of-proc) and for external/edge scenarios.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

src/windows/common/ConsoleState.cpp:68

• TryGetConsoleMode logs on every GetConsoleMode failure, but the surrounding helpers explicitly treat a disconnected console (e.g., ERROR_PIPE_NOT_CONNECTED) as an expected condition and avoid noisy logging. Since RestoreConsoleState calls this during teardown, this can produce unnecessary error logs during normal console shutdown/teardown paths.

Consider suppressing the log for ERROR_PIPE_NOT_CONNECTED (and possibly other expected disconnect errors) to match ChangeConsoleMode behavior.

std::optional<DWORD> TryGetConsoleMode(_In_ HANDLE Handle)
{
    DWORD mode{};
    if (!GetConsoleMode(Handle, &mode))
    {
        LOG_LAST_ERROR_MSG("GetConsoleMode failed");
        return std::nullopt;
    }

test/windows/wslc/WSLCCLIVTSupportUnitTests.cpp:65

• GetModuleFileNameW can return a truncated path when the buffer is too small (return value == buffer size). The current check (> 0) will treat a truncated path as valid, which can produce an incorrect candidate directory and make the test look for wsl.exe in the wrong place.

Consider rejecting the truncated-path case (or building the std::wstring from the returned length) and falling back to the System32 path when truncation occurs.

        std::array<wchar_t, MAX_PATH> modulePath{};
        if (GetModuleFileNameW(currentModule, modulePath.data(), static_cast<DWORD>(modulePath.size())) > 0)
        {
            std::wstring candidate{modulePath.data()};

[Copilot]

Pull request overview

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

[dkbennett]

ConsoleState is shared code, and the race this fixes (#41201) is specifically a wsl.exe problem: two overlapping wsl.exe clients sharing one inherited console and exiting out of order. WSLC's interactive paths (container attach/exec, session enter) are single foreground console owners, so they do not hit that race, but they do rely on ConsoleState unconditionally sanitizing the console on exit.

As written, the conditional restore changes behavior for every caller: if the console mode drifts from what a client configured, teardown now skips the restore. For WSLC that removes the guaranteed reset-on-exit and can leave a corrupted console (for example, echo or line input left disabled) after the CLI exits, with nothing else owning CONIN$ to repair it.

Proposal: gate the new behavior behind an explicit RestorePolicy that defaults to the existing unconditional restore, and opt only the wsl.exe sites into the conditional path.

enum class RestorePolicy
{
    // Always reapply saved state on teardown. Guarantees the console is sanitized on exit
    // even if the mode drifted mid-session. Correct for a sole console owner.
    Always,

    // Reapply only if the console still matches what this instance configured. Prevents
    // concurrent shared-console clients from restoring a stale temporary mode captured from
    // another client. Trades away the guaranteed reset.
    OnlyIfUnchanged,
};

explicit ConsoleState(RestorePolicy restorePolicy = RestorePolicy::Always);

// ... stored as:
RestorePolicy m_restorePolicy;

Restore branches on the policy, keeping the readback and configured tracking and just gating the check:

if (m_SavedInputMode.has_value())
{
    const bool restore = (m_restorePolicy == RestorePolicy::Always) ||
                         !m_ConfiguredInputMode.has_value() ||
                         (TryGetConsoleMode(m_InputHandle.get()) == m_ConfiguredInputMode);
    if (restore)
    {
        TrySetConsoleMode(m_InputHandle.get(), m_SavedInputMode.value());
    }

    m_SavedInputMode.reset();
    m_ConfiguredInputMode.reset();
}

The same shape applies to the input code page, output mode, and output code page.

Call sites:

Opt in to the concurrency fix (wsl.exe):

• src/windows/common/WslClient.cpp -> ConsoleState console{RestorePolicy::OnlyIfUnchanged};
• src/windows/common/svccomm.cpp -> ConsoleState Io{RestorePolicy::OnlyIfUnchanged};

Unchanged, default Always (guaranteed reset):

• src/windows/wslc/services/ContainerService.cpp
• src/windows/wslc/services/SessionService.cpp
• src/windows/wslc/services/ConsoleService.cpp

Notes:

• The default is the safe behavior, so existing WSLC call sites need no edits and keep today's guarantee.
• With this default, the two wsl.exe sites must opt in explicitly or the fix is inert. That is intended, and it makes the behavior change visible at the call site.
• WSLC can opt individual instances into OnlyIfUnchanged later if a real shared-console case appears, without another shared-code behavior change.

Also the tests need to be refactored into appropriate locations (2 are actual unit tests, 3 are functional, and they are common tests, not WSLC CLI tests), with the timing tests receiving a bit of scrutiny due to the potential to be flaky.

[Copilot]

Pull request overview

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

Suppressed comments (1)

test/windows/SimpleTests.cpp:180

• ResolveWslExecutablePath() throws if a co-located test-built wsl.exe is not found. That makes the new process-based ConsoleState tests brittle across build/test layouts (e.g., when wsl.exe is deployed to System32 or another staging directory but not placed next to the test module). Consider falling back to %SystemRoot%\System32\wsl.exe (or another existing “binary under test” locator) before failing, so the test can still run in environments where co-location isn’t guaranteed.

        THROW_HR_MSG(HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Could not find the co-located test-built wsl.exe");
    }

[shuaiyuanxx]

Hi @dkbennett Thanks for the detailed review. The requested changes are addressed:

• Added RestorePolicy, defaulting to Always to preserve WSLC behavior.
• Opted only WSL/WSLg process launches into OnlyIfUnchanged.
• Moved tests to UnitTests.cpp and SimpleTests.cpp.
• Replaced sleep-based timing with deterministic pipe synchronization.

The relevant builds and focused tests pass. Could you please take another look?

[Copilot]

Pull request overview

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

Suppressed comments (3)

test/windows/SimpleTests.cpp:189

• BuildControllableWslCommandLine() relies on printf ready on stdout as the readiness signal. Because stdout is a pipe here and there is no explicit autoflush, this can block up to PartialHandleRead’s 60s timeout on shells/programs that buffer stdout. Other tests in this repo explicitly enable autoflush (e.g. NetworkTests’ perl $|=1) for this reason.

    static std::wstring BuildControllableWslCommandLine()
    {
        // The child prints "ready" as soon as the Linux process is running,
        // then blocks on stdin so the parent can deterministically control its lifetime.
        const std::wstring arguments = L"-- sh -c \"printf ready; IFS= read -r _\"";
        const std::wstring wslPath = ResolveWslExecutablePath();
        return std::format(L"\"{}\" {}", wslPath, arguments);
    }

test/windows/SimpleTests.cpp:177

• ResolveWslExecutablePath() uses a fixed MAX_PATH buffer with GetModuleFileNameW and treats longer paths as a hard failure. This can make the test fail in long-path build layouts even when the co-located wsl.exe exists. Prefer the WIL helper (wil::GetModuleFileNameW) and std::filesystem path operations, which already appear in this repo’s tests and avoid MAX_PATH truncation.

        if (GetModuleHandleExW(
                GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
                reinterpret_cast<LPCWSTR>(&ResolveWslExecutablePath),
                &currentModule))
        {
            std::array<wchar_t, MAX_PATH> modulePath{};
            const auto modulePathLength = GetModuleFileNameW(currentModule, modulePath.data(), static_cast<DWORD>(modulePath.size()));
            if ((modulePathLength > 0) && (modulePathLength < modulePath.size()))
            {
                std::wstring candidate{modulePath.data(), modulePathLength};
                const auto separator = candidate.find_last_of(L"\\/");
                if (separator != std::wstring::npos)
                {
                    candidate.resize(separator + 1);
                    candidate += L"wsl.exe";

                    const auto attributes = GetFileAttributesW(candidate.c_str());
                    if ((attributes != INVALID_FILE_ATTRIBUTES) && !WI_IsFlagSet(attributes, FILE_ATTRIBUTE_DIRECTORY))
                    {
                        return candidate;
                    }
                }
            }

test/windows/UnitTests.cpp:7629

• The new RestorePolicy behavior is implemented for both input/output modes and input/output code pages, but the added regression tests here validate only CONIN$ mode behavior. This leaves the output-mode and code-page conditional-restore logic untested (e.g., external output mode drift, code page drift, and out-of-order restore for output state).

        DWORD finalMode{};
        VERIFY_WIN32_BOOL_SUCCEEDED(GetConsoleMode(conin.get(), &finalMode));
        VERIFY_ARE_EQUAL(
            baseline,
            finalMode,
            L"RestorePolicy::Always must restore the original mode even when the mode drifted after SetInteractiveMode");
    }

[dkbennett]

Removing my block since the parity concern has been resolved, I'm sure others may want to review this and the tests.

Changes addressed

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/windows/common/WslClient.cpp:53

• LaunchProcessOptions defaults RestorePolicySetting to Always. BashMain calls ParseLegacyArguments() and passes these options through to LaunchProcess() without overriding the policy, so bash.exe clients can still restore console state out-of-order (the original race) when multiple clients share a console. Consider defaulting this option to OnlyIfUnchanged so legacy/bash entrypoints get the safer behavior unless they explicitly opt into Always.

    RestorePolicy RestorePolicySetting = RestorePolicy::Always;