Bound WslCoreInstance init transactions

[Sylvain MOLINIER (SylvainM98)]

Summary of the Pull Request

SocketChannel defaults to an infinite timeout when none is supplied. Three WslCoreInstance operations rely on that default while holding locks needed by shutdown or other session operations. If the guest stays connected but stops responding, those operations can wait indefinitely and prevent WslService from making progress.

This passes the instance's existing m_socketTimeout to all three operations.

PR Checklist

• Closes: Closes WSLService can become unresponsive when init transactions wait indefinitely under session locks #41384
• 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 — no user-facing strings were changed
• Dev docs: Added/updated if needed — no developer documentation changes are required
• 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

On Windows, DefaultSocketTimeout is INFINITE, so a SocketChannel transaction that does not receive an explicit timeout can wait without bound.

WslCoreInstance already owns the applicable timeout value: m_socketTimeout, sourced from DistributionStartTimeout, which defaults to 60 seconds and is configurable through .wslconfig as wsl2.distributionStartTimeout.

The class already applies this timeout to comparable operations:

Operation	Bounded call
Instance creation result	ReceiveMessage(..., m_socketTimeout)
DrvFs mounting	Transaction(..., m_socketTimeout)
Instance termination	StartTransaction(m_socketTimeout)
Linux process creation	CreateLinuxProcess(..., m_socketTimeout)
Session creation	Transaction(..., m_socketTimeout)

The timeout was omitted from three operations:

Function	Lock held
CreateLxProcess()	WslCoreInstance::m_lock and the session-leader channel lock
Initialize()	WslCoreInstance::m_lock; its caller holds LxssUserSessionImpl::m_instanceLock
UpdateTimezone()	The init-channel lock; its caller holds m_instanceLock

A process-creation transaction can therefore hold WslCoreInstance::m_lock indefinitely. RequestStop() and Stop() require the same lock, while their callers hold LxssUserSessionImpl::m_instanceLock. This can prevent shutdown and other session operations from progressing.

If service or session teardown subsequently enters ClearSessionsAndBlockNewInstancesLockHeld(), it holds g_sessionTerminationLock while waiting for shutdown. New COM session activation can then also stop progressing.

This change passes m_socketTimeout to the three omitted operations. When the timeout expires, the affected operation fails, unwinds, and releases its locks. It does not change the timeout value or introduce a new policy.

This removes three unbounded waits. It does not address other independent causes of guest or service instability. Other unbounded m_miniInitChannel operations in WslCoreVm are intentionally left out because each requires a separate timeout and cancellation analysis.

Validation Steps Performed

Extended UnitTests::SocketChannel with two cases:

• A connected peer that remains open without replying makes the transaction fail with HRESULT_FROM_WIN32(ERROR_TIMEOUT). The elapsed time is checked to verify that the configured deadline was honored rather than the operation returning immediately.
• A peer that replies before the deadline completes normally and returns the expected payload.

Static inspection confirmed that the test helpers and assertions are available in the existing test scope.

The Windows build and tests were not run locally. Build, clang-format, and test validation are pending repository CI.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR threads a configurable socket timeout through init-channel transactions in the Windows service code and adds unit tests to validate transaction timeout and reply handling over SocketChannel.

Changes:

• Add unit tests covering SocketChannel::StartTransaction(timeout) timeout behavior and successful reply reception.
• Update WslCoreInstance init-channel calls to pass m_socketTimeout into transaction creation and execution.
• Ensure process creation transaction uses the configured socket timeout.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
test/windows/UnitTests.cpp	Adds new socket transaction tests for timeout and response behavior.
src/windows/service/exe/WslCoreInstance.cpp	Passes m_socketTimeout to init-channel transactions to bound socket operations.

---

💡 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 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (7)

test/windows/UnitTests.cpp:7718

• This test doesn’t actually assert that the StartTransaction(200) timeout is being honored: it only checks elapsed >= 100ms, so it could still pass if the implementation incorrectly uses a default (e.g., 100ms) and ignores the 200ms parameter. To make the test validate the intended behavior, assert elapsed time against the configured timeout (with a reasonable tolerance), and consider using named constants for the timeout and tolerances to keep the intent clear and reduce flakiness.

            auto transaction = channel.StartTransaction(200);

test/windows/UnitTests.cpp:7736

• This test doesn’t actually assert that the StartTransaction(200) timeout is being honored: it only checks elapsed >= 100ms, so it could still pass if the implementation incorrectly uses a default (e.g., 100ms) and ignores the 200ms parameter. To make the test validate the intended behavior, assert elapsed time against the configured timeout (with a reasonable tolerance), and consider using named constants for the timeout and tolerances to keep the intent clear and reduce flakiness.

            const auto start = std::chrono::steady_clock::now();
            const auto hr = wil::ResultFromException([&]() { transaction.Receive<RESULT_MESSAGE<int32_t>>(); });
            const auto elapsed =
                std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start).count();

            VERIFY_ARE_EQUAL(hr, HRESULT_FROM_WIN32(ERROR_TIMEOUT));
            VERIFY_IS_GREATER_THAN_OR_EQUAL(elapsed, 100LL);
            VERIFY_IS_LESS_THAN(elapsed, 5000LL);

test/windows/UnitTests.cpp:7718

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            auto transaction = channel.StartTransaction(200);

test/windows/UnitTests.cpp:7736

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            VERIFY_IS_GREATER_THAN_OR_EQUAL(elapsed, 100LL);
            VERIFY_IS_LESS_THAN(elapsed, 5000LL);

test/windows/UnitTests.cpp:7742

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            auto transaction = channel.StartTransaction(1000);

test/windows/UnitTests.cpp:7757

• The newly added tests introduce several hard-coded timing and value constants (e.g., 200, 1000, 100, 5000, 42). Using constexpr constants (e.g., kTimeoutMs, kMinElapsedMs, kMaxElapsedMs, kExpectedResult) would make the tests easier to understand and adjust, and would reduce the chance of mismatched expectations if timeouts are tweaked later.

            response.Result = 42;

src/windows/service/exe/WslCoreInstance.cpp:228

• Passing a raw nullptr as the middle argument makes the call site ambiguous (it’s not obvious what parameter is being intentionally omitted). If the API is expecting an optional/cancellation object, prefer an explicit empty value (e.g., std::nullopt / default-constructed token) or add a small clarifying comment at the call site so future readers don’t have to look up the overload/signature.

        port = sessionLeader->GetChannel().Transaction<LX_INIT_CREATE_PROCESS_UTILITY_VM>(messageSpan, nullptr, m_socketTimeout).Result;

[Copilot]

Pull request overview

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

Suppressed comments (2)

test/windows/UnitTests.cpp:7739

• elapsed is truncated to whole milliseconds via duration_cast(...).count(), which can make this assertion flaky (e.g., a ~199.9ms timeout becomes 199ms and fails the >= check). Consider keeping elapsed as a std::chrono::steady_clock::duration (or at least avoid truncation by using microseconds / rounding up) and compare using std::chrono::milliseconds durations directly.

            const auto start = std::chrono::steady_clock::now();
            const auto hr = wil::ResultFromException([&]() { transaction.Receive<RESULT_MESSAGE<int32_t>>(); });
            const auto elapsed =
                std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start).count();

            VERIFY_ARE_EQUAL(hr, HRESULT_FROM_WIN32(ERROR_TIMEOUT));
            VERIFY_IS_GREATER_THAN_OR_EQUAL(elapsed, static_cast<LONGLONG>(transactionTimeout) - schedulingTolerance);

src/windows/service/exe/WslCoreInstance.cpp:228

• Passing a raw nullptr as the middle argument makes the callsite ambiguous/brittle (it’s not self-documenting what parameter is being omitted). If possible, prefer an overload that takes only (messageSpan, timeout) (or uses a strongly-typed optional/default parameter) so readers don’t have to chase the callee signature; otherwise, add an explicit local variable (e.g., auto cancellation = /*...*/;) or a short comment indicating what nullptr represents.

        port = sessionLeader->GetChannel().Transaction<LX_INIT_CREATE_PROCESS_UTILITY_VM>(messageSpan, nullptr, m_socketTimeout).Result;

[Sylvain MOLINIER (SylvainM98)]

@microsoft-github-policy-service agree

[Blue (OneBlue)]

These tests don't seem to exercise the code that this PR changes, although I don't recommend having timing based tests like this, since they will fail if the machine is under pressure

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/windows/service/exe/WslCoreInstance.cpp:405

• PR description/issue notes say UnitTests::SocketChannel was extended with Transaction/StartTransaction timeout cases (peer stays connected without replying => HRESULT_FROM_WIN32(ERROR_TIMEOUT), and peer replying before deadline succeeds). I couldn't find any SocketChannel Transaction/StartTransaction timeout tests in the current test sources (only a ReceiveMessage finite-timeout test in test/windows/UnitTests.cpp around lines 7705-7713). Please add the claimed Transaction timeout coverage or update the PR description accordingly.

    auto config = wsl::windows::common::helpers::GenerateConfigurationMessage(
        m_configuration.Name, fixedDrives, m_defaultUid, timezone, {}, m_featureFlags, drvfsMount);

    auto transaction = m_initChannel->GetChannel().StartTransaction(m_socketTimeout);
    transaction.Send<LX_INIT_CONFIGURATION_INFORMATION>(gsl::span(config));