Fix concurrent console state restoration

[Shawn Yuan (shuaiyuanxx)]

Summary of the Pull Request

Fixes console-state races when concurrent wsl.exe clients share one Windows console. The WSL interactive state now remains active until the last participating client exits, then the original input/output modes and code pages are restored.

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

Details

This change coordinates clients directly with a per-console named mutex and shared file mapping:

• the first client captures the baseline and configures the console
• overlapping clients share the configured state
• only the last normally exiting client restores the baseline
• restoration remains conditional, preserving external console changes
• dead owner processes are pruned so later clients are not blocked

RestorePolicy::Always remains the default for WSLC and other existing callers. Coordinated behavior is limited to opted-in WSL launch paths, including WSLg, legacy bash, and debug shell.

Restoring the baseline after the sole remaining client is forcibly terminated remains outside scope because no client remains to restore process-local code pages. A later client can still acquire coordination without blocking.

Validation

• Built Debug common, wsl, wslservice, wslc, and wsltests
• Verified two overlapping clients with the Debug wsl.exe: the console remained configured after the first client exited and returned to baseline after the final client exited
• git diff --check passed

[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.

[David Bennett (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");
    }

[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");
    }

[David Bennett (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;

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/windows/common/ConsoleState.cpp:106

• MutexLock() collapses all unexpected WaitForSingleObject results into E_UNEXPECTED without surfacing the underlying Win32 failure (WAIT_FAILED). That makes diagnosing coordination failures difficult. Handle WAIT_FAILED explicitly with THROW_LAST_ERROR_IF so the root cause is preserved in telemetry/logs.

    explicit MutexLock(HANDLE mutex) : m_mutex(mutex)
    {
        const auto result = WaitForSingleObject(mutex, INFINITE);
        THROW_HR_IF(E_UNEXPECTED, (result != WAIT_OBJECT_0) && (result != WAIT_ABANDONED));
    }

src/windows/common/ConsoleState.cpp:325

• AcquireCoordination() throws when the fixed-size Owners table is full (ERROR_TOO_MANY_SESS). In practice this can turn an otherwise non-fatal console-state coordination issue into a hard failure to launch WSL. Consider treating table exhaustion as a best-effort failure: log and fall back to uncoordinated behavior instead of throwing.

    if (owner == std::end(state.Owners))
    {
        owner =
            std::find_if(std::begin(state.Owners), std::end(state.Owners), [](const auto& value) { return value.ProcessId == 0; });
        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_TOO_MANY_SESS), owner == std::end(state.Owners));
        *owner = {pid, 0, creationTime.value()};
    }

src/windows/common/ConsoleState.h:31

• The PR description mentions "RestorePolicy::Always" as the default, but the code defines RestorePolicy::{Exclusive, Cooperative} and defaults to Exclusive. Please align the PR description (or naming) so reviewers/users don’t look for a non-existent enum value.

// Controls when ConsoleState attempts to restore the original console state.
enum class RestorePolicy
{
    // Exclusive restore to the original state captured by this instance.
    Exclusive,

    // Restore only when the state still matches what this instance configured.
    Cooperative,
};

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/windows/common/ConsoleState.cpp:343

• AcquireCoordination() hard-fails when all 128 owner slots are in use (THROW_HR_IF(ERROR_TOO_MANY_SESS)). This introduces a new, arbitrary limit where wsl.exe can fail to start even though it could safely fall back to non-coordinated restore behavior for this console.

        owner =
            std::find_if(std::begin(state.Owners), std::end(state.Owners), [](const auto& value) { return value.ProcessId == 0; });
        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_TOO_MANY_SESS), owner == std::end(state.Owners));
        *owner = {pid, 0, creationTime.value()};

src/windows/common/ConsoleState.cpp:316

• AcquireCoordination() uses THROW_LAST_ERROR_IF on CreateMutexW/CreateFileMappingW/MapViewOfFile. Since RestorePolicy::Cooperative is an opt-in best-effort coordination path, failing to create these OS objects should fall back to the non-coordinated ConsoleState behavior instead of aborting the WSL client with an exception.

This issue also appears on line 340 of the same file.

    const auto name = std::format(L"Local\\WSL.ConsoleState.v1.{:X}", consoleId.value());
    m_coordinationMutex.reset(CreateMutexW(nullptr, FALSE, (name + L".Mutex").c_str()));
    THROW_LAST_ERROR_IF(!m_coordinationMutex);
    m_coordinationMapping.reset(
        CreateFileMappingW(INVALID_HANDLE_VALUE, nullptr, PAGE_READWRITE, 0, sizeof(CoordinationState), (name + L".Mapping").c_str()));
    THROW_LAST_ERROR_IF(!m_coordinationMapping);
    auto view = MapViewOfFile(m_coordinationMapping.get(), FILE_MAP_ALL_ACCESS, 0, 0, sizeof(CoordinationState));
    THROW_LAST_ERROR_IF(!view);
    auto unmap = wil::scope_exit([&] { LOG_IF_WIN32_BOOL_FALSE(UnmapViewOfFile(view)); });

[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/UnitTests.cpp:525

• The newly added WSL2 test SharedMountSurvivesDistroTermination appears unrelated to this PR’s stated goal (fixing concurrent Windows console state restoration for overlapping wsl.exe clients). Mixing an unrelated mount/systemd regression test into this PR makes review and potential rollback harder; consider moving this test to a separate PR focused on mount guards/systemd shutdown behavior.

    WSL2_TEST_METHOD(SharedMountSurvivesDistroTermination)
    {
        constexpr auto peerDistroName = L"mount-guard-peer-test";

        auto validate = [&](const std::string& automountRoot) {

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/windows/common/ConsoleState.cpp:212

• In Cooperative mode, AcquireCoordination() can throw (e.g., CreateMutexW/CreateFileMappingW/MapViewOfFile failures or ERROR_TOO_MANY_SESS). That would make interactive launches fail outright instead of falling back to the existing best-effort per-instance behavior. Coordination should be opportunistic: if it can’t be acquired, continue with the normal SetInteractiveMode path rather than surfacing an exception.

    if ((m_restorePolicy == RestorePolicy::Cooperative) && AcquireCoordination())
    {
        m_interactiveModeConfigured = true;
        return;
    }

test/windows/SimpleTests.cpp:167

• Naming: unique_kill_process is a struct type, but type names in this repo follow PascalCase (see coding guidelines). Consider renaming it (and its constructors/usages) to something like UniqueKillProcess to align with existing conventions.

    struct unique_kill_process
    {
        unique_kill_process() = default;

        explicit unique_kill_process(wil::unique_handle&& process) : m_process(std::move(process))

[Copilot]

Pull request overview

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

Suppressed comments (2)

test/windows/SimpleTests.cpp:223

• ConsoleSnapshot/GetConsoleSnapshot/SetConsoleSnapshot/VerifyConsoleSnapshot are duplicated here and in test/windows/UnitTests.cpp. Keeping two copies increases the chance they drift (e.g., if fields are added/changed). Consider moving these helpers to a shared test utility (e.g., Common.h/.cpp or a dedicated ConsoleStateTestHelpers.h) and reusing them from both test suites.

    struct ConsoleSnapshot
    {
        DWORD InputMode;
        UINT InputCodePage;
        DWORD OutputMode;
        UINT OutputCodePage;
    };

    static ConsoleSnapshot GetConsoleSnapshot(HANDLE conin, HANDLE conout)

test/windows/UnitTests.cpp:7707

• The restore guard in VerifyExternalInputModeRestore() only resets CONIN$ input mode (SetConsoleMode) and does not restore code pages / CONOUT$ mode. If this test fails partway through (or if ConsoleState restoration regresses), it can leave the console code pages/modes modified and cause follow-on tests to run in a corrupted console state. Capture/restore a full snapshot (including CONOUT$ and code pages) using the existing helper functions, and assert the full baseline is restored.

        DWORD baseline{};
        VERIFY_WIN32_BOOL_SUCCEEDED(GetConsoleMode(conin.get(), &baseline));
        auto restoreBaseline = wil::scope_exit([&] { ::SetConsoleMode(conin.get(), baseline); });

        std::optional<DWORD> externalMode;

[Blue (OneBlue)]

Shawn Yuan (@shuaiyuanxx): Looking at the original issue, I'm not sure that the complexity that this change brings is worth it.

Running multiple processes under the same TTY in parallel is inherently going to lead to race conditions, since there will be a TOCTOU between reading the current terminal's state and deciding whether it should be changed or not.

If a user wants to do something like this, the easiest solution would just be to not run multiple interactive processes in the same terminal session, and use pipes or CREATE_NEW_CONSOLE

[Blue (OneBlue)]

I don't recommend doing this. This will lead to surprising results when running the tests locally.

We should have the same behavior as the other tests: Use the installed version of wsl.exe