From ba81b75d6f654dd3a8190fcfdb76f2ef9ee02aba Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 5 Mar 2026 11:09:04 -0800 Subject: [PATCH 01/15] Add hourly upstream sync workflow for feature/wsl-for-apps --- .github/workflows/sync-upstream.yml | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 .github/workflows/sync-upstream.yml diff --git a/.github/workflows/sync-upstream.yml b/.github/workflows/sync-upstream.yml new file mode 100644 index 0000000000..fb0c13bfca --- /dev/null +++ b/.github/workflows/sync-upstream.yml @@ -0,0 +1,49 @@ +name: Sync fork with upstream + +on: + schedule: + # Hourly during Pacific business hours (8 AM - 6 PM PT = 16:00 - 02:00 UTC next day) + - cron: '0 16-23 * * 1-5' # Mon-Fri 16:00-23:00 UTC (8 AM - 3 PM PT) + - cron: '0 0-1 * * 2-6' # Tue-Sat 00:00-01:00 UTC (4 PM - 5 PM PT prev day) + workflow_dispatch: # Allow manual trigger + +jobs: + sync: + runs-on: ubuntu-latest + steps: + - name: Checkout fork + uses: actions/checkout@v4 + with: + ref: feature/wsl-for-apps + fetch-depth: 0 + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Add upstream remote + run: git remote add upstream https://github.com/Microsoft/WSL.git + + - name: Fetch upstream + run: git fetch upstream feature/wsl-for-apps + + - name: Check if update needed + id: check + run: | + LOCAL=$(git rev-parse HEAD) + UPSTREAM=$(git rev-parse upstream/feature/wsl-for-apps) + echo "local=$LOCAL" >> "$GITHUB_OUTPUT" + echo "upstream=$UPSTREAM" >> "$GITHUB_OUTPUT" + if [ "$LOCAL" = "$UPSTREAM" ]; then + echo "needs_update=false" >> "$GITHUB_OUTPUT" + echo "Already up to date." + else + echo "needs_update=true" >> "$GITHUB_OUTPUT" + echo "Update needed: $LOCAL -> $UPSTREAM" + fi + + - name: Fast-forward merge + if: steps.check.outputs.needs_update == 'true' + run: | + git merge --ff-only upstream/feature/wsl-for-apps + + - name: Push changes + if: steps.check.outputs.needs_update == 'true' + run: git push origin feature/wsl-for-apps From 50f9ce0c5c8b5b200b670fd3c707cfd1eb7f3f15 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Thu, 5 Mar 2026 11:19:36 -0800 Subject: [PATCH 02/15] Remove upstream sync workflow --- .github/workflows/sync-upstream.yml | 49 ----------------------------- 1 file changed, 49 deletions(-) delete mode 100644 .github/workflows/sync-upstream.yml diff --git a/.github/workflows/sync-upstream.yml b/.github/workflows/sync-upstream.yml deleted file mode 100644 index fb0c13bfca..0000000000 --- a/.github/workflows/sync-upstream.yml +++ /dev/null @@ -1,49 +0,0 @@ -name: Sync fork with upstream - -on: - schedule: - # Hourly during Pacific business hours (8 AM - 6 PM PT = 16:00 - 02:00 UTC next day) - - cron: '0 16-23 * * 1-5' # Mon-Fri 16:00-23:00 UTC (8 AM - 3 PM PT) - - cron: '0 0-1 * * 2-6' # Tue-Sat 00:00-01:00 UTC (4 PM - 5 PM PT prev day) - workflow_dispatch: # Allow manual trigger - -jobs: - sync: - runs-on: ubuntu-latest - steps: - - name: Checkout fork - uses: actions/checkout@v4 - with: - ref: feature/wsl-for-apps - fetch-depth: 0 - token: ${{ secrets.GITHUB_TOKEN }} - - - name: Add upstream remote - run: git remote add upstream https://github.com/Microsoft/WSL.git - - - name: Fetch upstream - run: git fetch upstream feature/wsl-for-apps - - - name: Check if update needed - id: check - run: | - LOCAL=$(git rev-parse HEAD) - UPSTREAM=$(git rev-parse upstream/feature/wsl-for-apps) - echo "local=$LOCAL" >> "$GITHUB_OUTPUT" - echo "upstream=$UPSTREAM" >> "$GITHUB_OUTPUT" - if [ "$LOCAL" = "$UPSTREAM" ]; then - echo "needs_update=false" >> "$GITHUB_OUTPUT" - echo "Already up to date." - else - echo "needs_update=true" >> "$GITHUB_OUTPUT" - echo "Update needed: $LOCAL -> $UPSTREAM" - fi - - - name: Fast-forward merge - if: steps.check.outputs.needs_update == 'true' - run: | - git merge --ff-only upstream/feature/wsl-for-apps - - - name: Push changes - if: steps.check.outputs.needs_update == 'true' - run: git push origin feature/wsl-for-apps From f2a524520be5045889b1083d05d7576f598d72dd Mon Sep 17 00:00:00 2001 From: kvega005 Date: Wed, 22 Jul 2026 11:12:09 -0700 Subject: [PATCH 03/15] Use transitions to manage container lifecycle operations --- .../wslcsession/DockerEventTracker.cpp | 8 +- src/windows/wslcsession/DockerEventTracker.h | 1 + src/windows/wslcsession/WSLCContainer.cpp | 230 +++++++++++------- src/windows/wslcsession/WSLCContainer.h | 39 +-- 4 files changed, 175 insertions(+), 103 deletions(-) diff --git a/src/windows/wslcsession/DockerEventTracker.cpp b/src/windows/wslcsession/DockerEventTracker.cpp index f5018d3bca..6d38b08f07 100644 --- a/src/windows/wslcsession/DockerEventTracker.cpp +++ b/src/windows/wslcsession/DockerEventTracker.cpp @@ -153,7 +153,11 @@ void DockerEventTracker::OnEvent(const std::string_view& event) void DockerEventTracker::OnContainerEvent(const nlohmann::json& parsed, const std::string& action, std::uint64_t eventTime) { static std::map events{ - {"start", ContainerEvent::Start}, {"die", ContainerEvent::Stop}, {"destroy", ContainerEvent::Destroy}, {"exec_die", ContainerEvent::ExecDied}}; + {"start", ContainerEvent::Start}, + {"die", ContainerEvent::Stop}, + {"destroy", ContainerEvent::Destroy}, + {"exec_die", ContainerEvent::ExecDied}, + {"restart", ContainerEvent::Restart}}; auto actor = parsed.find("Actor"); THROW_HR_IF_MSG(E_INVALIDARG, actor == parsed.end(), "Missing Actor in container event"); @@ -299,4 +303,4 @@ void DockerEventTracker::UnregisterCallback(size_t Id) noexcept auto volumeRemove = std::ranges::remove_if(m_volumeCallbacks, [Id](auto& entry) { return entry.CallbackId == Id; }); WI_ASSERT(volumeRemove.size() == 1); m_volumeCallbacks.erase(volumeRemove.begin(), volumeRemove.end()); -} \ No newline at end of file +} diff --git a/src/windows/wslcsession/DockerEventTracker.h b/src/windows/wslcsession/DockerEventTracker.h index e040c7592f..9a5a6a2aca 100644 --- a/src/windows/wslcsession/DockerEventTracker.h +++ b/src/windows/wslcsession/DockerEventTracker.h @@ -26,6 +26,7 @@ enum class ContainerEvent { Create, Start, + Restart, Stop, Exit, Destroy, diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 24c7399932..6458723e23 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -36,6 +36,7 @@ using wsl::windows::common::io::OverlappedIOHandle; using wsl::windows::common::io::ReadHandle; using wsl::windows::common::io::RelayHandle; using wsl::windows::service::wslc::ContainerPortMapping; +using wsl::windows::service::wslc::DockerEventTracker; using wsl::windows::service::wslc::DockerHTTPClient; using wsl::windows::service::wslc::DockerHTTPException; using wsl::windows::service::wslc::IWSLCVolume; @@ -741,9 +742,12 @@ void WSLCContainerImpl::Attach(LPCSTR DetachKeys, WSLCHandle* Stdin, WSLCHandle* void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessStartOptions* StartOptions) { - // Acquire an exclusive lock since this method modifies m_initProcessControl, m_initProcess and m_state. + std::shared_ptr transition; + auto transitionGuard = m_transitionLock.lock_exclusive(); auto lock = m_lock.lock_exclusive(); + WI_ASSERT(!m_transition); + THROW_HR_WITH_USER_ERROR_IF(WSLC_E_CONTAINER_IS_RUNNING, Localization::MessageWslcContainerIsRunning(m_id), m_state == WslcContainerStateRunning); THROW_HR_IF_MSG( @@ -794,12 +798,14 @@ void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessSt auto control = std::make_unique(*this, m_dockerClient); - std::lock_guard processesLock{m_processesLock}; - m_initProcessControl = control.get(); - - m_initProcess = wil::MakeOrThrow(std::move(control), std::move(io), m_initProcessFlags); + { + std::lock_guard processesLock{m_processesLock}; + m_initProcessControl = control.get(); + m_initProcess = wil::MakeOrThrow(std::move(control), std::move(io), m_initProcessFlags); + } auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [this]() mutable { + std::lock_guard processesLock{m_processesLock}; m_initProcess.Reset(); m_initProcessControl = nullptr; }); @@ -826,9 +832,6 @@ void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessSt auto portCleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [this]() { UnmapPorts(); }); MapPorts(); - m_stopNotification.Event.ResetEvent(); - m_stopNotification.EventTime.store(0, std::memory_order_relaxed); - try { m_dockerClient.StartContainer(m_id, detachKeys); @@ -873,54 +876,92 @@ void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessSt } } + transition = std::make_shared(ContainerEvent::Start); + WI_ASSERT(!m_transition); + m_transition = transition; + + lock.reset(); + WaitForTransition(transition); + portCleanup.release(); volumeCleanup.release(); - - Transition(WslcContainerStateRunning); cleanup.release(); } -void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime) +void WSLCContainerImpl::WaitForTransition(const std::shared_ptr& transition) const { - // We must release m_lock and m_stopLock before the wrapper's destructor calls - // Disconnect(), so in-flight COM callers can drain from COMImplClass::m_callers. - unique_com_disconnect comWrapper; + THROW_HR_IF_MSG( + E_UNEXPECTED, + !m_wslcSession.WaitForEventOrSessionTerminating(transition->Completed.get(), std::chrono::milliseconds{INFINITE}), + "Unexpected lifecycle transition timeout for container '%hs'", + m_id.c_str()); - if (event == ContainerEvent::Stop) + WI_ASSERT(transition->Completed.is_signaled()); + + if (transition->Exception) { - THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value()); - SetExitCode(exitCode.value()); + std::rethrow_exception(transition->Exception); + } +} - std::unique_lock stopGuard{m_stopLock, std::try_to_lock}; +__requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::CompleteTransition(const std::shared_ptr& transition, std::exception_ptr exception) noexcept +{ + WI_ASSERT(m_transition == transition); + transition->Exception = std::move(exception); + m_transition.reset(); + transition->Completed.SetEvent(); +} + +void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime) noexcept +{ + // The wrapper must be disconnected after m_lock is released so in-flight COM callers can drain. + unique_com_disconnect comWrapper; + auto lock = m_lock.lock_exclusive(); + auto transition = m_transition; - m_stopNotification.EventTime.store(eventTime, std::memory_order_release); - m_stopNotification.Event.SetEvent(); + // TODO - Only WSLC should start the container, so if we receive a start event, it must be expected by a transition. + // Otherwise the container was started externally. Consider logging or emitting a warning if the container was started + // externally. + if (event == ContainerEvent::Start && transition && transition->ExpectedEvent == ContainerEvent::Start) + { + CommitState(WslcContainerStateRunning, eventTime); + CompleteTransition(transition); + } + else if (event == ContainerEvent::Stop) + { + THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value()); - // If Stop() is already in flight, it will wake when the stop event is signaled and take care of cleanup. - if (!stopGuard.owns_lock()) + // A Stop while expecting Start should not occur normally: Docker emits start before die, and the event stream processes + // them serially. It would indicate external manipulation. Ignoring it avoids applying an old exit code to the newly + // staged init process. + if (transition && (transition->ExpectedEvent == ContainerEvent::Start)) { - return; + WSL_LOG( + "UnexpectedContainerExit", TraceLoggingValue(m_id.c_str(), "Id"), TraceLoggingValue(exitCode.value(), "ExitCode")); + } + else + { + SetExitCode(exitCode.value()); + OnStopped(eventTime); } - - auto lock = m_lock.lock_exclusive(); - comWrapper = OnStopped(eventTime); } else if (event == ContainerEvent::Destroy) { - WI_ASSERT(!m_destroyEvent.is_signaled()); - m_destroyEvent.SetEvent(); - - auto lock = m_lock.lock_exclusive(); - if (m_state != WslcContainerStateDeleted) { - Transition(WslcContainerStateDeleted, eventTime); + CommitState(WslcContainerStateDeleted, eventTime); comWrapper = ReleaseResources(); } - // Signal init exit after the state transition so awaiters observe state=Deleted - // (and any post-delete cleanup) rather than the prior Running/Exited state. + // Signal init exit after the state transition and resource cleanup so awaiters observe Deleted. SignalInitProcessExit(); + + if (transition) + { + WI_ASSERT(transition->ExpectedEvent == ContainerEvent::Destroy); + transition->Wrapper = std::move(comWrapper); + CompleteTransition(transition); + } } WSL_LOG( @@ -932,12 +973,22 @@ void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCod void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) { - // N.B. comWrapper must be destructed after m_lock and m_stopLock are released. - unique_com_disconnect comWrapper; + std::shared_ptr transition; - std::unique_lock stopGuard{m_stopLock}; + // Stop callers take the transition lock shared. m_lock serializes the first caller's + // Docker request and publication of m_transition; later callers copy that transition + // and wait on the same completion event. Start and Delete take this lock exclusively. + auto transitionGuard = m_transitionLock.lock_shared(); auto lock = m_lock.lock_exclusive(); + if (m_transition) + { + transition = m_transition; + lock.reset(); + WaitForTransition(transition); + return; + } + if (m_state == WslcContainerStateExited && !Kill) { return; @@ -969,11 +1020,6 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) if (Kill) { m_dockerClient.SignalContainer(m_id, SignalArg); - - if (!waitForStop) - { - return; - } } else { @@ -995,29 +1041,19 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) } } - // Wait for the stop event to get the Docker timestamp. - std::optional stopTimestamp; - if (m_wslcSession.WaitForEventOrSessionTerminating(m_stopNotification.Event.get(), 60s)) + if (waitForStop) { - stopTimestamp = m_stopNotification.EventTime.load(std::memory_order_acquire); - } + transition = std::make_shared(ContainerEvent::Stop); + WI_ASSERT(!m_transition); + m_transition = transition; - comWrapper = OnStopped(stopTimestamp); - - if (WI_IsFlagSet(m_containerFlags, WSLCContainerFlagsRm)) - { - // Release locks before waiting on the docker destroy event: OnEvent(Destroy) takes m_lock, - // and the wrapper's destructor (Disconnect) must run after locks are released. lock.reset(); - stopGuard.unlock(); - m_wslcSession.WaitForEventOrSessionTerminating(m_destroyEvent.get(), 60s); + WaitForTransition(transition); } } -__requires_exclusive_lock_held(m_lock) unique_com_disconnect WSLCContainerImpl::OnStopped(std::optional stopTimestamp) +__requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(std::optional stopTimestamp) { - unique_com_disconnect comWrapper; - // Notify plugin manager that the container is stopping. Errors are ignored. if (m_state == WslcContainerStateRunning) { @@ -1031,46 +1067,71 @@ __requires_exclusive_lock_held(m_lock) unique_com_disconnect WSLCContainerImpl:: ReleaseProcesses(); ReleaseRuntimeResources(); - // Only drive state transition + auto-delete if we're still Running. A concurrent - // Delete() may have already moved us to Deleted. + // Ignore duplicate or late Stop events so they do not overwrite an already committed state. if (m_state == WslcContainerStateRunning) { - Transition(WslcContainerStateExited, stopTimestamp); + CommitState(WslcContainerStateExited, stopTimestamp); + } + + auto transition = m_transition; + std::exception_ptr transitionException; + + // Docker delete request is already sent. + if (transition && transition->ExpectedEvent == ContainerEvent::Destroy) + { + return; + } + + // Stop with Rm must initiate Delete. + if (WI_IsFlagSet(m_containerFlags, WSLCContainerFlagsRm)) + { + try + { + m_dockerClient.DeleteContainer(m_id, true, true); - if (WI_IsFlagSet(m_containerFlags, WSLCContainerFlagsRm)) + if (transition) + { + transition->ExpectedEvent = ContainerEvent::Destroy; + } + + return; + } + catch (...) { - comWrapper = DeleteExclusiveLockHeld(WSLCDeleteFlagsForce | WSLCDeleteFlagsDeleteVolumes); + transitionException = std::current_exception(); + LOG_CAUGHT_EXCEPTION_MSG("Failed to remove container '%hs'", m_id.c_str()); } } - // For the Rm path, defer init-exit signaling to OnEvent(Destroy) so callers waiting - // on init exit observe destroy-side cleanup first. - if (WI_IsFlagClear(m_containerFlags, WSLCContainerFlagsRm)) + SignalInitProcessExit(); + + if (transition) { - SignalInitProcessExit(); + CompleteTransition(transition, std::move(transitionException)); } - - return comWrapper; } void WSLCContainerImpl::Delete(WSLCDeleteFlags Flags) { - // N.B. wrapper must be destroyed after m_lock is released, since its destructor calls Disconnect(). - unique_com_disconnect wrapper; - { - auto lock = m_lock.lock_exclusive(); - wrapper = DeleteExclusiveLockHeld(Flags); - } + std::shared_ptr transition; + auto transitionGuard = m_transitionLock.lock_exclusive(); + auto lock = m_lock.lock_exclusive(); - // Wait for the docker destroy event so anonymous volume cleanup is reflected in tracking by - // the time we return. - if (WI_IsFlagSet(Flags, WSLCDeleteFlagsDeleteVolumes)) - { - m_wslcSession.WaitForEventOrSessionTerminating(m_destroyEvent.get(), 60s); - } + WI_ASSERT(!m_transition); + + RequestDeleteExclusiveLockHeld(Flags); + + transition = std::make_shared(ContainerEvent::Destroy); + WI_ASSERT(!m_transition); + m_transition = transition; + + lock.reset(); + WaitForTransition(transition); + + lock = m_lock.lock_exclusive(); } -__requires_exclusive_lock_held(m_lock) unique_com_disconnect WSLCContainerImpl::DeleteExclusiveLockHeld(WSLCDeleteFlags Flags) +__requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::RequestDeleteExclusiveLockHeld(WSLCDeleteFlags Flags) { // Validate that the container is not running or already deleted. THROW_HR_WITH_USER_ERROR_IF( @@ -1088,9 +1149,6 @@ __requires_exclusive_lock_held(m_lock) unique_com_disconnect WSLCContainerImpl:: m_dockerClient.DeleteContainer(m_id, WI_IsFlagSet(Flags, WSLCDeleteFlagsForce), WI_IsFlagSet(Flags, WSLCDeleteFlagsDeleteVolumes)); } CATCH_AND_THROW_DOCKER_USER_ERROR("Failed to delete container '%hs'", m_id.c_str()); - - Transition(WslcContainerStateDeleted); - return ReleaseResources(); } void WSLCContainerImpl::Export(WSLCHandle OutHandle) const @@ -2355,7 +2413,7 @@ __requires_exclusive_lock_held(m_lock) unique_com_disconnect WSLCContainerImpl:: return unique_com_disconnect{std::exchange(m_comWrapper, nullptr)}; } -__requires_lock_held(m_lock) void WSLCContainerImpl::Transition(WSLCContainerState State, std::optional stateChangedAt) noexcept +__requires_lock_held(m_lock) void WSLCContainerImpl::CommitState(WSLCContainerState State, std::optional stateChangedAt) noexcept { // N.B. A deleted container cannot transition back to any other state. WI_ASSERT(m_state != WslcContainerStateDeleted); diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index 23cb60e786..b89b6e89eb 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -122,7 +122,7 @@ class WSLCContainerImpl : public std::enable_shared_from_this WSLCContainerState State() const noexcept; std::vector GetPorts() const; - __requires_lock_held(m_lock) void Transition(WSLCContainerState State, std::optional stateChangedAt = std::nullopt) noexcept; + __requires_lock_held(m_lock) void CommitState(WSLCContainerState State, std::optional stateChangedAt = std::nullopt) noexcept; const std::string& ID() const noexcept; @@ -158,17 +158,33 @@ class WSLCContainerImpl : public std::enable_shared_from_this IORelay& Relay); private: - __requires_exclusive_lock_held(m_lock) [[nodiscard]] unique_com_disconnect DeleteExclusiveLockHeld(WSLCDeleteFlags Flags); + struct StateTransition + { + StateTransition(ContainerEvent expectedEvent) : ExpectedEvent(expectedEvent) + { + } + + ContainerEvent ExpectedEvent; + wil::unique_event Completed{wil::EventOptions::ManualReset}; + std::exception_ptr Exception; + unique_com_disconnect Wrapper; + }; + + __requires_exclusive_lock_held(m_lock) void RequestDeleteExclusiveLockHeld(WSLCDeleteFlags Flags); void AllocateBridgedModePorts(); - void OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime); + void OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime) noexcept; + + void WaitForTransition(const std::shared_ptr& transition) const; + __requires_exclusive_lock_held(m_lock) void CompleteTransition( + const std::shared_ptr& transition, std::exception_ptr exception = {}) noexcept; __requires_exclusive_lock_held(m_lock) [[nodiscard]] unique_com_disconnect ReleaseResources(); __requires_exclusive_lock_held(m_lock) void ReleaseRuntimeResources(); __requires_exclusive_lock_held(m_lock) void ReleaseProcesses(); __requires_exclusive_lock_held(m_lock) [[nodiscard]] unique_com_disconnect PrepareDisconnectComWrapper(); - __requires_exclusive_lock_held(m_lock) [[nodiscard]] unique_com_disconnect OnStopped(std::optional stopTimestamp); + __requires_exclusive_lock_held(m_lock) void OnStopped(std::optional stopTimestamp); void SetExitCode(int ExitCode) noexcept; void SignalInitProcessExit() noexcept; @@ -193,17 +209,10 @@ class WSLCContainerImpl : public std::enable_shared_from_this __guarded_by(m_processesLock) Microsoft::WRL::ComPtr m_initProcess; __guarded_by(m_processesLock) DockerContainerProcessControl* m_initProcessControl = nullptr; - struct StopNotification - { - std::atomic EventTime{0}; - wil::unique_event Event{wil::EventOptions::None}; - } m_stopNotification; - - wil::unique_event m_destroyEvent{wil::EventOptions::ManualReset}; - - // Serializes Stop() callers and signals OnEvent that a Stop is in flight. - // Must be acquired before m_lock when both are needed. - std::mutex m_stopLock; + // Stop callers hold this lock shared so concurrent requests can join the same + // transition. Other lifecycle transitions hold it exclusively. + wil::srwlock m_transitionLock; + _Guarded_by_(m_lock) std::shared_ptr m_transition; DockerHTTPClient& m_dockerClient; std::uint64_t m_stateChangedAt{static_cast(std::time(nullptr))}; From 5fa5aebc81e5f9edaabe179efc732c60df3c2c5b Mon Sep 17 00:00:00 2001 From: kvega005 Date: Wed, 22 Jul 2026 11:43:52 -0700 Subject: [PATCH 04/15] Log on unexpected event --- src/windows/wslcsession/WSLCContainer.cpp | 47 +++++++++++++---------- src/windows/wslcsession/WSLCContainer.h | 2 +- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 6458723e23..c2dc1f794d 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -919,32 +919,25 @@ void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCod auto lock = m_lock.lock_exclusive(); auto transition = m_transition; - // TODO - Only WSLC should start the container, so if we receive a start event, it must be expected by a transition. - // Otherwise the container was started externally. Consider logging or emitting a warning if the container was started - // externally. - if (event == ContainerEvent::Start && transition && transition->ExpectedEvent == ContainerEvent::Start) + if (event == ContainerEvent::Start) { - CommitState(WslcContainerStateRunning, eventTime); - CompleteTransition(transition); - } - else if (event == ContainerEvent::Stop) - { - THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value()); - - // A Stop while expecting Start should not occur normally: Docker emits start before die, and the event stream processes - // them serially. It would indicate external manipulation. Ignoring it avoids applying an old exit code to the newly - // staged init process. - if (transition && (transition->ExpectedEvent == ContainerEvent::Start)) + // Only WSLC should start the container, so if we receive a start event, it must be expected by a transition. + // Otherwise the container was started externally. Log if the container was started externally. + if (transition && transition->ExpectedEvent == ContainerEvent::Start) { - WSL_LOG( - "UnexpectedContainerExit", TraceLoggingValue(m_id.c_str(), "Id"), TraceLoggingValue(exitCode.value(), "ExitCode")); + CommitState(WslcContainerStateRunning, eventTime); + CompleteTransition(transition); } else { - SetExitCode(exitCode.value()); - OnStopped(eventTime); + WSL_LOG("UnexpectedContainerStart", TraceLoggingValue(m_id.c_str(), "Id")); } } + else if (event == ContainerEvent::Stop) + { + THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value()); + OnStopped(exitCode.value(), eventTime); + } else if (event == ContainerEvent::Destroy) { if (m_state != WslcContainerStateDeleted) @@ -1052,8 +1045,21 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) } } -__requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(std::optional stopTimestamp) +__requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(int exitCode, std::optional stopTimestamp) { + auto transition = m_transition; + + // A Stop while expecting Start should not occur normally: Docker emits start before die, and the event stream processes + // them serially. It would indicate external manipulation. Ignoring it avoids applying an old exit code to the newly + // staged init process. + if (transition && (transition->ExpectedEvent == ContainerEvent::Start)) + { + WSL_LOG("UnexpectedContainerExit", TraceLoggingValue(m_id.c_str(), "Id"), TraceLoggingValue(exitCode, "ExitCode")); + return; + } + + SetExitCode(exitCode); + // Notify plugin manager that the container is stopping. Errors are ignored. if (m_state == WslcContainerStateRunning) { @@ -1073,7 +1079,6 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(std::op CommitState(WslcContainerStateExited, stopTimestamp); } - auto transition = m_transition; std::exception_ptr transitionException; // Docker delete request is already sent. diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index b89b6e89eb..2debe0366d 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -184,7 +184,7 @@ class WSLCContainerImpl : public std::enable_shared_from_this __requires_exclusive_lock_held(m_lock) void ReleaseProcesses(); __requires_exclusive_lock_held(m_lock) [[nodiscard]] unique_com_disconnect PrepareDisconnectComWrapper(); - __requires_exclusive_lock_held(m_lock) void OnStopped(std::optional stopTimestamp); + __requires_exclusive_lock_held(m_lock) void OnStopped(int exitCode, std::optional stopTimestamp); void SetExitCode(int ExitCode) noexcept; void SignalInitProcessExit() noexcept; From e6e1a6ec7020a519a3d49c944b4c93e316391b7d Mon Sep 17 00:00:00 2001 From: kvega005 Date: Wed, 22 Jul 2026 11:50:47 -0700 Subject: [PATCH 05/15] Add assert --- src/windows/wslcsession/WSLCContainer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index c2dc1f794d..45de26e7ec 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -925,6 +925,7 @@ void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCod // Otherwise the container was started externally. Log if the container was started externally. if (transition && transition->ExpectedEvent == ContainerEvent::Start) { + WI_ASSERT(m_state == WslcContainerStateCreated || m_state == WslcContainerStateExited); CommitState(WslcContainerStateRunning, eventTime); CompleteTransition(transition); } From 1b31af05c1a7ddcd16ec5e2e9dfd08841cda465a Mon Sep 17 00:00:00 2001 From: kvega005 Date: Wed, 22 Jul 2026 12:13:50 -0700 Subject: [PATCH 06/15] Address feedback about vague code --- src/windows/wslcsession/WSLCContainer.cpp | 25 ++++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 45de26e7ec..35bd3242da 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -1120,21 +1120,26 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(int exi void WSLCContainerImpl::Delete(WSLCDeleteFlags Flags) { std::shared_ptr transition; - auto transitionGuard = m_transitionLock.lock_exclusive(); - auto lock = m_lock.lock_exclusive(); - WI_ASSERT(!m_transition); + { + auto transitionGuard = m_transitionLock.lock_exclusive(); + auto lock = m_lock.lock_exclusive(); - RequestDeleteExclusiveLockHeld(Flags); + WI_ASSERT(!m_transition); - transition = std::make_shared(ContainerEvent::Destroy); - WI_ASSERT(!m_transition); - m_transition = transition; + RequestDeleteExclusiveLockHeld(Flags); - lock.reset(); - WaitForTransition(transition); + transition = std::make_shared(ContainerEvent::Destroy); + WI_ASSERT(!m_transition); + m_transition = transition; - lock = m_lock.lock_exclusive(); + lock.reset(); + WaitForTransition(transition); + + // Reacquire m_lock to wait for OnEvent() to leave its critical section. Both locks must be released before transition + // destroys the COM wrapper. + lock = m_lock.lock_exclusive(); + } } __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::RequestDeleteExclusiveLockHeld(WSLCDeleteFlags Flags) From 0afa44c32200c30788c88c7a370f71b340239b17 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Wed, 22 Jul 2026 12:51:09 -0700 Subject: [PATCH 07/15] Add tests --- test/windows/WSLCTests.cpp | 88 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index edc13644b6..353bd61d50 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -6826,6 +6826,94 @@ class WSLCTests } } + WSLC_TEST_METHOD(ConcurrentContainerStopAndKill) + { + WSLCContainerLauncher launcher( + "debian:latest", + "test-concurrent-container-stops", + {"/bin/sh", "-c", "trap 'echo stopping; read value; exit 0' TERM; echo ready; while true; do sleep 1; done"}, + {}, + "host", + WSLCProcessFlagsStdin); + + auto container = launcher.Launch(*m_defaultSession); + auto initProcess = container.GetInitProcess(); + auto input = initProcess.GetStdHandle(0); + auto outputHandle = initProcess.GetStdHandle(1); + PartialHandleRead output{outputHandle.get()}; + output.ExpectConsume("ready\n"); + + HRESULT stopResult{}; + HRESULT killResult{}; + std::thread stopThread; + std::thread killThread; + + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { + input.reset(); + + if (stopThread.joinable()) + { + stopThread.join(); + } + + if (killThread.joinable()) + { + killThread.join(); + } + }); + + stopThread = std::thread([&]() { stopResult = container.Get().Stop(WSLCSignalSIGTERM, WSLC_STOP_TIMEOUT_NONE); }); + + output.ExpectConsume("stopping\n"); + + // Kill fails after the container exits, so success proves it joined the active Stop transition. + wil::unique_event killStarted{wil::EventOptions::ManualReset}; + killThread = std::thread([&]() { + killStarted.SetEvent(); + killResult = container.Get().Kill(WSLCSignalSIGTERM); + }); + + VERIFY_IS_TRUE(killStarted.wait(30 * 1000)); + VERIFY_ARE_EQUAL(WaitForSingleObject(killThread.native_handle(), 100), WAIT_TIMEOUT); + + const char stopInput = '\n'; + DWORD bytesWritten{}; + VERIFY_WIN32_BOOL_SUCCEEDED(WriteFile(input.get(), &stopInput, sizeof(stopInput), &bytesWritten, nullptr)); + VERIFY_ARE_EQUAL(bytesWritten, static_cast(sizeof(stopInput))); + input.reset(); + + VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + VERIFY_ARE_EQUAL(WaitForSingleObject(killThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + + stopThread.join(); + killThread.join(); + cleanup.release(); + + VERIFY_SUCCEEDED(stopResult); + VERIFY_SUCCEEDED(killResult); + VERIFY_ARE_EQUAL(container.State(), WslcContainerStateExited); + } + + WSLC_TEST_METHOD(ForceDeleteAutoRemoveContainer) + { + WSLCContainerLauncher launcher("debian:latest", "test-force-delete-auto-remove", {"sleep", "99999"}); + launcher.SetContainerFlags(WSLCContainerFlagsRm); + + auto container = launcher.Launch(*m_defaultSession); + auto id = container.Id(); + auto name = container.Name(); + + VERIFY_SUCCEEDED(container.Get().Delete(WSLCDeleteFlagsForce)); + container.SetDeleteOnClose(false); + + VERIFY_ARE_EQUAL(container.State(), WslcContainerStateDeleted); + VERIFY_ARE_EQUAL(container.Get().Delete(WSLCDeleteFlagsForce), RPC_E_DISCONNECTED); + + wil::com_ptr openedContainer; + VERIFY_ARE_EQUAL(m_defaultSession->OpenContainer(id.c_str(), &openedContainer), WSLC_E_CONTAINER_NOT_FOUND); + VERIFY_ARE_EQUAL(m_defaultSession->OpenContainer(name.c_str(), &openedContainer), WSLC_E_CONTAINER_NOT_FOUND); + } + WSLC_TEST_METHOD(ContainerListFilter) { // Lists containers with the given filter options and returns the names as a set. From 6cd6be8583dd10bfa244249e65899a5651fb9347 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Wed, 22 Jul 2026 15:55:49 -0700 Subject: [PATCH 08/15] Fix plugin test --- test/windows/PluginTests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/windows/PluginTests.cpp b/test/windows/PluginTests.cpp index 9abfcaaedc..8b7cea0301 100644 --- a/test/windows/PluginTests.cpp +++ b/test/windows/PluginTests.cpp @@ -721,6 +721,7 @@ class PluginTests WSLC Image created, session=*, id=sha256:*, name=wslc-registry:latest WSLC Container started, session=*, id=*, name=*, image=wslc-registry:latest, state=running WSLC Image created, session=*, id=sha256:*, name=127.0.0.1:5000/debian:latest + WSLC Container stopping, session=*, id=* WSLC Session stopping, name=plugin-wslc-pull-test, id=*)"; ValidateLogFile(ExpectedOutput); From 17109c630d17ea2e33a6c348157a8effac623239 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Wed, 22 Jul 2026 16:09:06 -0700 Subject: [PATCH 09/15] Rename transition guard --- src/windows/wslcsession/WSLCContainer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 35bd3242da..dc4cf2feb6 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -743,7 +743,7 @@ void WSLCContainerImpl::Attach(LPCSTR DetachKeys, WSLCHandle* Stdin, WSLCHandle* void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessStartOptions* StartOptions) { std::shared_ptr transition; - auto transitionGuard = m_transitionLock.lock_exclusive(); + auto transitionLock = m_transitionLock.lock_exclusive(); auto lock = m_lock.lock_exclusive(); WI_ASSERT(!m_transition); @@ -972,7 +972,7 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) // Stop callers take the transition lock shared. m_lock serializes the first caller's // Docker request and publication of m_transition; later callers copy that transition // and wait on the same completion event. Start and Delete take this lock exclusively. - auto transitionGuard = m_transitionLock.lock_shared(); + auto transitionLock = m_transitionLock.lock_shared(); auto lock = m_lock.lock_exclusive(); if (m_transition) @@ -1122,7 +1122,7 @@ void WSLCContainerImpl::Delete(WSLCDeleteFlags Flags) std::shared_ptr transition; { - auto transitionGuard = m_transitionLock.lock_exclusive(); + auto transitionLock = m_transitionLock.lock_exclusive(); auto lock = m_lock.lock_exclusive(); WI_ASSERT(!m_transition); From 3e981fa12fd6a69aa0995bdbbe9bcde1f3c60a7b Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 24 Jul 2026 11:19:25 -0700 Subject: [PATCH 10/15] Address feedback about disconnect deadlock and no throw in on event --- src/windows/wslcsession/WSLCContainer.cpp | 127 ++++++++++++---------- 1 file changed, 67 insertions(+), 60 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index dc4cf2feb6..1bd7ece075 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -936,7 +936,7 @@ void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCod } else if (event == ContainerEvent::Stop) { - THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value()); + WI_ASSERT(exitCode.has_value()); OnStopped(exitCode.value(), eventTime); } else if (event == ContainerEvent::Destroy) @@ -969,80 +969,87 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) { std::shared_ptr transition; - // Stop callers take the transition lock shared. m_lock serializes the first caller's - // Docker request and publication of m_transition; later callers copy that transition - // and wait on the same completion event. Start and Delete take this lock exclusively. - auto transitionLock = m_transitionLock.lock_shared(); - auto lock = m_lock.lock_exclusive(); - - if (m_transition) - { - transition = m_transition; - lock.reset(); - WaitForTransition(transition); - return; - } - - if (m_state == WslcContainerStateExited && !Kill) - { - return; - } - else if (m_state != WslcContainerStateRunning) - { - THROW_HR_WITH_USER_ERROR_MSG( - WSLC_E_CONTAINER_NOT_RUNNING, - Localization::MessageWslcContainerNotRunning(m_id), - "Cannot stop container '%hs', state: %i", - m_id.c_str(), - m_state); - } - - std::optional SignalArg; - if (Signal != WSLCSignalNone) { - SignalArg = Signal; - } + // Stop callers take the transition lock shared. m_lock serializes the first caller's + // Docker request and publication of m_transition; later callers copy that transition + // and wait on the same completion event. Start and Delete take this lock exclusively. + auto transitionLock = m_transitionLock.lock_shared(); + auto lock = m_lock.lock_exclusive(); - ValidateStopTimeout(TimeoutSeconds, true); + if (m_transition) + { + transition = m_transition; + lock.reset(); + WaitForTransition(transition); + return; + } - // Don't wait for the container to stop if we're not sending SIGKILL, since it may not stop the container. - // N.B. If the signal was SIGTERM for instance, we'll receive the stop notification via OnEvent(). - bool waitForStop = !Kill || (SignalArg.value_or(WSLCSignalSIGKILL) == WSLCSignalSIGKILL); + if (m_state == WslcContainerStateExited && !Kill) + { + return; + } + else if (m_state != WslcContainerStateRunning) + { + THROW_HR_WITH_USER_ERROR_MSG( + WSLC_E_CONTAINER_NOT_RUNNING, + Localization::MessageWslcContainerNotRunning(m_id), + "Cannot stop container '%hs', state: %i", + m_id.c_str(), + m_state); + } - try - { - if (Kill) + std::optional SignalArg; + if (Signal != WSLCSignalNone) { - m_dockerClient.SignalContainer(m_id, SignalArg); + SignalArg = Signal; } - else + + ValidateStopTimeout(TimeoutSeconds, true); + + // Don't wait for the container to stop if we're not sending SIGKILL, since it may not stop the container. + // N.B. If the signal was SIGTERM for instance, we'll receive the stop notification via OnEvent(). + bool waitForStop = !Kill || (SignalArg.value_or(WSLCSignalSIGKILL) == WSLCSignalSIGKILL); + + try { - std::optional TimeoutArg; - if (TimeoutSeconds != WSLC_STOP_TIMEOUT_DEFAULT) + if (Kill) { - TimeoutArg = TimeoutSeconds; + m_dockerClient.SignalContainer(m_id, SignalArg); } + else + { + std::optional TimeoutArg; - m_dockerClient.StopContainer(m_id, SignalArg, TimeoutArg); + if (TimeoutSeconds != WSLC_STOP_TIMEOUT_DEFAULT) + { + TimeoutArg = TimeoutSeconds; + } + + m_dockerClient.StopContainer(m_id, SignalArg, TimeoutArg); + } } - } - catch (const DockerHTTPException& e) - { - // HTTP 304 is returned when the container is already stopped. - if (Kill || e.StatusCode() != 304) + catch (const DockerHTTPException& e) { - THROW_DOCKER_USER_ERROR_MSG(e, "Failed to %hs container '%hs'", Kill ? "kill" : "stop", m_id.c_str()); + // HTTP 304 is returned when the container is already stopped. + if (Kill || e.StatusCode() != 304) + { + THROW_DOCKER_USER_ERROR_MSG(e, "Failed to %hs container '%hs'", Kill ? "kill" : "stop", m_id.c_str()); + } } - } - if (waitForStop) - { - transition = std::make_shared(ContainerEvent::Stop); - WI_ASSERT(!m_transition); - m_transition = transition; + if (waitForStop) + { + transition = std::make_shared(ContainerEvent::Stop); - lock.reset(); - WaitForTransition(transition); + WI_ASSERT(!m_transition); + m_transition = transition; + + lock.reset(); + WaitForTransition(transition); + + // Wait for OnEvent() to leave its critical section before transition can destroy the COM wrapper. + lock = m_lock.lock_exclusive(); + } } } From de2cac6ef748491550c641a9c6a368aad93652df Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 24 Jul 2026 11:50:52 -0700 Subject: [PATCH 11/15] Allow transitions to outlive COM calls --- src/windows/wslcsession/WSLCContainer.cpp | 140 ++++++++++++---------- src/windows/wslcsession/WSLCContainer.h | 19 ++- 2 files changed, 90 insertions(+), 69 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 1bd7ece075..aa62d1ba1c 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -743,10 +743,8 @@ void WSLCContainerImpl::Attach(LPCSTR DetachKeys, WSLCHandle* Stdin, WSLCHandle* void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessStartOptions* StartOptions) { std::shared_ptr transition; - auto transitionLock = m_transitionLock.lock_exclusive(); auto lock = m_lock.lock_exclusive(); - - WI_ASSERT(!m_transition); + WaitForConflictingTransitionToComplete(lock); THROW_HR_WITH_USER_ERROR_IF(WSLC_E_CONTAINER_IS_RUNNING, Localization::MessageWslcContainerIsRunning(m_id), m_state == WslcContainerStateRunning); @@ -876,19 +874,34 @@ void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessSt } } - transition = std::make_shared(ContainerEvent::Start); + transition = std::make_shared(TransitionKind::Start, ContainerEvent::Start); WI_ASSERT(!m_transition); m_transition = transition; - lock.reset(); - WaitForTransition(transition); - portCleanup.release(); volumeCleanup.release(); cleanup.release(); + + lock.reset(); + AttachToTransition(transition); +} + +void WSLCContainerImpl::WaitForConflictingTransitionToComplete(wil::rwlock_release_exclusive_scope_exit& lock, std::optional kind) +{ + while (m_transition && (!kind.has_value() || m_transition->Kind != kind.value())) + { + // The transition may own the COM wrapper, so release this reference before reacquiring m_lock. + { + auto transition = m_transition; + lock.reset(); + WaitForTransitionCompletion(transition); + } + + lock = m_lock.lock_exclusive(); + } } -void WSLCContainerImpl::WaitForTransition(const std::shared_ptr& transition) const +void WSLCContainerImpl::WaitForTransitionCompletion(const std::shared_ptr& transition) const { THROW_HR_IF_MSG( E_UNEXPECTED, @@ -897,6 +910,11 @@ void WSLCContainerImpl::WaitForTransition(const std::shared_ptr m_id.c_str()); WI_ASSERT(transition->Completed.is_signaled()); +} + +void WSLCContainerImpl::AttachToTransition(const std::shared_ptr& transition) const +{ + WaitForTransitionCompletion(transition); if (transition->Exception) { @@ -914,55 +932,60 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::CompleteTransitio void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime) noexcept { - // The wrapper must be disconnected after m_lock is released so in-flight COM callers can drain. + // The com wrapper can not be disconnected with lock held. Destroy event may transfer ownership of the disconnect + // to an awaiting COM caller via the transition object. So both must be destroyed after releasing the lock. unique_com_disconnect comWrapper; - auto lock = m_lock.lock_exclusive(); - auto transition = m_transition; + std::shared_ptr transition; - if (event == ContainerEvent::Start) { - // Only WSLC should start the container, so if we receive a start event, it must be expected by a transition. - // Otherwise the container was started externally. Log if the container was started externally. - if (transition && transition->ExpectedEvent == ContainerEvent::Start) + auto lock = m_lock.lock_exclusive(); + transition = m_transition; + + if (event == ContainerEvent::Start) { - WI_ASSERT(m_state == WslcContainerStateCreated || m_state == WslcContainerStateExited); - CommitState(WslcContainerStateRunning, eventTime); - CompleteTransition(transition); + // Only WSLC should start the container, so if we receive a start event, it must be expected by a transition. + // Otherwise the container was started externally. Log if the container was started externally. + if (transition && transition->ExpectedEvent == ContainerEvent::Start) + { + WI_ASSERT(m_state == WslcContainerStateCreated || m_state == WslcContainerStateExited); + CommitState(WslcContainerStateRunning, eventTime); + CompleteTransition(transition); + } + else + { + WSL_LOG("UnexpectedContainerStart", TraceLoggingValue(m_id.c_str(), "Id")); + } } - else + else if (event == ContainerEvent::Stop) { - WSL_LOG("UnexpectedContainerStart", TraceLoggingValue(m_id.c_str(), "Id")); + WI_ASSERT(exitCode.has_value()); + OnStopped(exitCode.value(), eventTime); } - } - else if (event == ContainerEvent::Stop) - { - WI_ASSERT(exitCode.has_value()); - OnStopped(exitCode.value(), eventTime); - } - else if (event == ContainerEvent::Destroy) - { - if (m_state != WslcContainerStateDeleted) + else if (event == ContainerEvent::Destroy) { - CommitState(WslcContainerStateDeleted, eventTime); - comWrapper = ReleaseResources(); - } + if (m_state != WslcContainerStateDeleted) + { + CommitState(WslcContainerStateDeleted, eventTime); + comWrapper = ReleaseResources(); + } - // Signal init exit after the state transition and resource cleanup so awaiters observe Deleted. - SignalInitProcessExit(); + // Signal init exit after the state transition and resource cleanup so awaiters observe Deleted. + SignalInitProcessExit(); - if (transition) - { - WI_ASSERT(transition->ExpectedEvent == ContainerEvent::Destroy); - transition->Wrapper = std::move(comWrapper); - CompleteTransition(transition); + if (transition) + { + WI_ASSERT(transition->ExpectedEvent == ContainerEvent::Destroy); + transition->Wrapper = std::move(comWrapper); + CompleteTransition(transition); + } } - } - WSL_LOG( - "ContainerEvent", - TraceLoggingValue(m_name.c_str(), "Name"), - TraceLoggingValue(m_id.c_str(), "Id"), - TraceLoggingValue((int)event, "Event")); + WSL_LOG( + "ContainerEvent", + TraceLoggingValue(m_name.c_str(), "Name"), + TraceLoggingValue(m_id.c_str(), "Id"), + TraceLoggingValue((int)event, "Event")); + } } void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) @@ -970,17 +993,15 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) std::shared_ptr transition; { - // Stop callers take the transition lock shared. m_lock serializes the first caller's - // Docker request and publication of m_transition; later callers copy that transition - // and wait on the same completion event. Start and Delete take this lock exclusively. - auto transitionLock = m_transitionLock.lock_shared(); auto lock = m_lock.lock_exclusive(); + WaitForConflictingTransitionToComplete(lock, TransitionKind::Stop); if (m_transition) { + WI_ASSERT(m_transition->Kind == TransitionKind::Stop); transition = m_transition; lock.reset(); - WaitForTransition(transition); + AttachToTransition(transition); return; } @@ -1039,16 +1060,13 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) if (waitForStop) { - transition = std::make_shared(ContainerEvent::Stop); + transition = std::make_shared(TransitionKind::Stop, ContainerEvent::Stop); WI_ASSERT(!m_transition); m_transition = transition; lock.reset(); - WaitForTransition(transition); - - // Wait for OnEvent() to leave its critical section before transition can destroy the COM wrapper. - lock = m_lock.lock_exclusive(); + AttachToTransition(transition); } } } @@ -1129,23 +1147,17 @@ void WSLCContainerImpl::Delete(WSLCDeleteFlags Flags) std::shared_ptr transition; { - auto transitionLock = m_transitionLock.lock_exclusive(); auto lock = m_lock.lock_exclusive(); - - WI_ASSERT(!m_transition); + WaitForConflictingTransitionToComplete(lock); RequestDeleteExclusiveLockHeld(Flags); - transition = std::make_shared(ContainerEvent::Destroy); + transition = std::make_shared(TransitionKind::Delete, ContainerEvent::Destroy); WI_ASSERT(!m_transition); m_transition = transition; lock.reset(); - WaitForTransition(transition); - - // Reacquire m_lock to wait for OnEvent() to leave its critical section. Both locks must be released before transition - // destroys the COM wrapper. - lock = m_lock.lock_exclusive(); + AttachToTransition(transition); } } diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index 2debe0366d..49cb7ce70a 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -158,12 +158,20 @@ class WSLCContainerImpl : public std::enable_shared_from_this IORelay& Relay); private: + enum class TransitionKind + { + Start, + Stop, + Delete + }; + struct StateTransition { - StateTransition(ContainerEvent expectedEvent) : ExpectedEvent(expectedEvent) + StateTransition(TransitionKind kind, ContainerEvent expectedEvent) : Kind(kind), ExpectedEvent(expectedEvent) { } + const TransitionKind Kind; ContainerEvent ExpectedEvent; wil::unique_event Completed{wil::EventOptions::ManualReset}; std::exception_ptr Exception; @@ -175,7 +183,11 @@ class WSLCContainerImpl : public std::enable_shared_from_this void AllocateBridgedModePorts(); void OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime) noexcept; - void WaitForTransition(const std::shared_ptr& transition) const; + // Returns with lock held when no transition is active or the active transition matches kind. + void WaitForConflictingTransitionToComplete(wil::rwlock_release_exclusive_scope_exit& lock, std::optional kind = std::nullopt); + void WaitForTransitionCompletion(const std::shared_ptr& transition) const; + void AttachToTransition(const std::shared_ptr& transition) const; + __requires_exclusive_lock_held(m_lock) void CompleteTransition( const std::shared_ptr& transition, std::exception_ptr exception = {}) noexcept; @@ -209,9 +221,6 @@ class WSLCContainerImpl : public std::enable_shared_from_this __guarded_by(m_processesLock) Microsoft::WRL::ComPtr m_initProcess; __guarded_by(m_processesLock) DockerContainerProcessControl* m_initProcessControl = nullptr; - // Stop callers hold this lock shared so concurrent requests can join the same - // transition. Other lifecycle transitions hold it exclusively. - wil::srwlock m_transitionLock; _Guarded_by_(m_lock) std::shared_ptr m_transition; DockerHTTPClient& m_dockerClient; From ffceff217d19590428955f3fb6b4686d011b81cf Mon Sep 17 00:00:00 2001 From: kvega005 Date: Mon, 27 Jul 2026 11:26:33 -0700 Subject: [PATCH 12/15] Fix deadlock --- src/windows/wslcsession/WSLCContainer.cpp | 15 ++++++++++++--- src/windows/wslcsession/WSLCContainer.h | 1 + 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index aa62d1ba1c..e857c7dd48 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -890,7 +890,6 @@ void WSLCContainerImpl::WaitForConflictingTransitionToComplete(wil::rwlock_relea { while (m_transition && (!kind.has_value() || m_transition->Kind != kind.value())) { - // The transition may own the COM wrapper, so release this reference before reacquiring m_lock. { auto transition = m_transition; lock.reset(); @@ -916,6 +915,14 @@ void WSLCContainerImpl::AttachToTransition(const std::shared_ptrWrapper); + } + if (transition->Exception) { std::rethrow_exception(transition->Exception); @@ -932,8 +939,7 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::CompleteTransitio void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime) noexcept { - // The com wrapper can not be disconnected with lock held. Destroy event may transfer ownership of the disconnect - // to an awaiting COM caller via the transition object. So both must be destroyed after releasing the lock. + // Either owner may disconnect the COM wrapper, so both must outlive m_lock. unique_com_disconnect comWrapper; std::shared_ptr transition; @@ -975,7 +981,10 @@ void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCod if (transition) { WI_ASSERT(transition->ExpectedEvent == ContainerEvent::Destroy); + + // Let a COM caller waiting on this transition perform the disconnect, avoiding a deadlock with OnEvent. transition->Wrapper = std::move(comWrapper); + CompleteTransition(transition); } } diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index 49cb7ce70a..d336ac7d46 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -175,6 +175,7 @@ class WSLCContainerImpl : public std::enable_shared_from_this ContainerEvent ExpectedEvent; wil::unique_event Completed{wil::EventOptions::ManualReset}; std::exception_ptr Exception; + // Access under WSLCContainerImpl::m_lock. unique_com_disconnect Wrapper; }; From 9121724dd8af4e44e3adebf25c60860f8b86f76e Mon Sep 17 00:00:00 2001 From: kvega005 Date: Mon, 27 Jul 2026 13:02:49 -0700 Subject: [PATCH 13/15] Cleanup --- src/windows/wslcsession/WSLCContainer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index e857c7dd48..77a235cba2 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -904,7 +904,7 @@ void WSLCContainerImpl::WaitForTransitionCompletion(const std::shared_ptrCompleted.get(), std::chrono::milliseconds{INFINITE}), + !m_wslcSession.WaitForEventOrSessionTerminating(transition->Completed.get(), 60s), "Unexpected lifecycle transition timeout for container '%hs'", m_id.c_str()); @@ -1085,8 +1085,8 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(int exi auto transition = m_transition; // A Stop while expecting Start should not occur normally: Docker emits start before die, and the event stream processes - // them serially. It would indicate external manipulation. Ignoring it avoids applying an old exit code to the newly - // staged init process. + // them serially. It would indicate external manipulation. Ignoring it avoids applying an old exit code to the newly + // staged init process. if (transition && (transition->ExpectedEvent == ContainerEvent::Start)) { WSL_LOG("UnexpectedContainerExit", TraceLoggingValue(m_id.c_str(), "Id"), TraceLoggingValue(exitCode, "ExitCode")); From e5eeb153cdad5a8de49f8626ce6f6910eb026d8c Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 31 Jul 2026 10:24:37 -0700 Subject: [PATCH 14/15] Address feedback --- src/windows/wslcsession/WSLCContainer.cpp | 172 +++++++++++++--------- src/windows/wslcsession/WSLCContainer.h | 11 +- test/windows/WSLCTests.cpp | 72 +++++++-- 3 files changed, 171 insertions(+), 84 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 77a235cba2..ed5ce2c976 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -743,8 +743,9 @@ void WSLCContainerImpl::Attach(LPCSTR DetachKeys, WSLCHandle* Stdin, WSLCHandle* void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessStartOptions* StartOptions) { std::shared_ptr transition; + auto lifecycleLock = m_lifecycleGate.lock_exclusive(); auto lock = m_lock.lock_exclusive(); - WaitForConflictingTransitionToComplete(lock); + WaitForConflictingTransitionToComplete(lock, lifecycleLock); THROW_HR_WITH_USER_ERROR_IF(WSLC_E_CONTAINER_IS_RUNNING, Localization::MessageWslcContainerIsRunning(m_id), m_state == WslcContainerStateRunning); @@ -874,39 +875,58 @@ void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessSt } } - transition = std::make_shared(TransitionKind::Start, ContainerEvent::Start); - WI_ASSERT(!m_transition); - m_transition = transition; + transition = StartTransition(TransitionKind::Start, ContainerEvent::Start); portCleanup.release(); volumeCleanup.release(); cleanup.release(); lock.reset(); + lifecycleLock.reset(); AttachToTransition(transition); } -void WSLCContainerImpl::WaitForConflictingTransitionToComplete(wil::rwlock_release_exclusive_scope_exit& lock, std::optional kind) +template +void WSLCContainerImpl::WaitForConflictingTransitionToComplete( + wil::rwlock_release_exclusive_scope_exit& lock, TLifecycleLock& lifecycleLock, std::optional kind) { while (m_transition && (!kind.has_value() || m_transition->Kind != kind.value())) { { auto transition = m_transition; lock.reset(); + lifecycleLock.reset(); WaitForTransitionCompletion(transition); } + if constexpr (std::is_same_v) + { + lifecycleLock = m_lifecycleGate.lock_shared(); + } + else + { + static_assert(std::is_same_v); + lifecycleLock = m_lifecycleGate.lock_exclusive(); + } + lock = m_lock.lock_exclusive(); } } +__requires_exclusive_lock_held(m_lock) std::shared_ptr WSLCContainerImpl::StartTransition( + TransitionKind kind, ContainerEvent expectedEvent) +{ + auto transition = std::make_shared(kind, expectedEvent); + WI_ASSERT(!m_transition); + m_transition = transition; + return transition; +} + void WSLCContainerImpl::WaitForTransitionCompletion(const std::shared_ptr& transition) const { - THROW_HR_IF_MSG( - E_UNEXPECTED, - !m_wslcSession.WaitForEventOrSessionTerminating(transition->Completed.get(), 60s), - "Unexpected lifecycle transition timeout for container '%hs'", - m_id.c_str()); + auto io = m_wslcSession.CreateIOContext(); + io.AddHandle(std::make_unique(transition->Completed.get())); + io.Run({}); WI_ASSERT(transition->Completed.is_signaled()); } @@ -944,6 +964,7 @@ void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCod std::shared_ptr transition; { + auto lifecycleLock = m_lifecycleGate.lock_exclusive(); auto lock = m_lock.lock_exclusive(); transition = m_transition; @@ -1002,24 +1023,23 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) std::shared_ptr transition; { + auto lifecycleLock = m_lifecycleGate.lock_shared(); auto lock = m_lock.lock_exclusive(); - WaitForConflictingTransitionToComplete(lock, TransitionKind::Stop); + WaitForConflictingTransitionToComplete(lock, lifecycleLock, TransitionKind::Stop); - if (m_transition) - { - WI_ASSERT(m_transition->Kind == TransitionKind::Stop); - transition = m_transition; - lock.reset(); - AttachToTransition(transition); - return; - } + transition = m_transition; + WI_ASSERT(!transition || transition->Kind == TransitionKind::Stop); - if (m_state == WslcContainerStateExited && !Kill) - { - return; - } - else if (m_state != WslcContainerStateRunning) + // There can be an active stop transition post observing the exited state for cases where additional work needs to be done + // after the contaier stopped: e.g. auto remove, restart, etc. Therefore, if there is an active stop transition, we still + // need to attach to it below. This check simply skips creating a new transition once the state is already exited. + if (!transition && m_state != WslcContainerStateRunning) { + if (m_state == WslcContainerStateExited && !Kill) + { + return; + } + THROW_HR_WITH_USER_ERROR_MSG( WSLC_E_CONTAINER_NOT_RUNNING, Localization::MessageWslcContainerNotRunning(m_id), @@ -1027,57 +1047,69 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) m_id.c_str(), m_state); } - - std::optional SignalArg; - if (Signal != WSLCSignalNone) + // This check ensures WSLC does not call into docker if it has already observed the exited state. This prevents + // conflicting with scenarios where work needs to be done after the container exits. + else if (m_state == WslcContainerStateRunning) { - SignalArg = Signal; - } - - ValidateStopTimeout(TimeoutSeconds, true); + std::optional SignalArg; - // Don't wait for the container to stop if we're not sending SIGKILL, since it may not stop the container. - // N.B. If the signal was SIGTERM for instance, we'll receive the stop notification via OnEvent(). - bool waitForStop = !Kill || (SignalArg.value_or(WSLCSignalSIGKILL) == WSLCSignalSIGKILL); - - try - { - if (Kill) + if (Signal != WSLCSignalNone) { - m_dockerClient.SignalContainer(m_id, SignalArg); + SignalArg = Signal; } - else - { - std::optional TimeoutArg; - if (TimeoutSeconds != WSLC_STOP_TIMEOUT_DEFAULT) + ValidateStopTimeout(TimeoutSeconds, true); + + // Don't wait for the container to stop if we're not sending SIGKILL, since it may not stop the container. + // N.B. If the signal was SIGTERM for instance, we'll receive the stop notification via OnEvent(). + bool waitForStop = !Kill || (SignalArg.value_or(WSLCSignalSIGKILL) == WSLCSignalSIGKILL); + + lock.reset(); + + try + { + if (Kill) { - TimeoutArg = TimeoutSeconds; + m_dockerClient.SignalContainer(m_id, SignalArg); } + else + { + std::optional TimeoutArg; + + if (TimeoutSeconds != WSLC_STOP_TIMEOUT_DEFAULT) + { + TimeoutArg = TimeoutSeconds; + } - m_dockerClient.StopContainer(m_id, SignalArg, TimeoutArg); + m_dockerClient.StopContainer(m_id, SignalArg, TimeoutArg); + } } - } - catch (const DockerHTTPException& e) - { - // HTTP 304 is returned when the container is already stopped. - if (Kill || e.StatusCode() != 304) + catch (const DockerHTTPException& e) { - THROW_DOCKER_USER_ERROR_MSG(e, "Failed to %hs container '%hs'", Kill ? "kill" : "stop", m_id.c_str()); + // HTTP 304 is returned when the container is already stopped. + if (Kill || e.StatusCode() != 304) + { + THROW_DOCKER_USER_ERROR_MSG(e, "Failed to %hs container '%hs'", Kill ? "kill" : "stop", m_id.c_str()); + } } - } - if (waitForStop) - { - transition = std::make_shared(TransitionKind::Stop, ContainerEvent::Stop); - - WI_ASSERT(!m_transition); - m_transition = transition; + if (waitForStop) + { + lock = m_lock.lock_exclusive(); + transition = m_transition; - lock.reset(); - AttachToTransition(transition); + if (!transition) + { + transition = StartTransition(TransitionKind::Stop, ContainerEvent::Stop); + } + } } } + + if (transition) + { + AttachToTransition(transition); + } } __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(int exitCode, std::optional stopTimestamp) @@ -1154,20 +1186,16 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(int exi void WSLCContainerImpl::Delete(WSLCDeleteFlags Flags) { std::shared_ptr transition; + auto lifecycleLock = m_lifecycleGate.lock_exclusive(); + auto lock = m_lock.lock_exclusive(); + WaitForConflictingTransitionToComplete(lock, lifecycleLock); - { - auto lock = m_lock.lock_exclusive(); - WaitForConflictingTransitionToComplete(lock); - - RequestDeleteExclusiveLockHeld(Flags); - - transition = std::make_shared(TransitionKind::Delete, ContainerEvent::Destroy); - WI_ASSERT(!m_transition); - m_transition = transition; + RequestDeleteExclusiveLockHeld(Flags); + transition = StartTransition(TransitionKind::Delete, ContainerEvent::Destroy); - lock.reset(); - AttachToTransition(transition); - } + lock.reset(); + lifecycleLock.reset(); + AttachToTransition(transition); } __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::RequestDeleteExclusiveLockHeld(WSLCDeleteFlags Flags) diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index d336ac7d46..c1514fe958 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -184,8 +184,13 @@ class WSLCContainerImpl : public std::enable_shared_from_this void AllocateBridgedModePorts(); void OnEvent(ContainerEvent event, std::optional exitCode, std::uint64_t eventTime) noexcept; - // Returns with lock held when no transition is active or the active transition matches kind. - void WaitForConflictingTransitionToComplete(wil::rwlock_release_exclusive_scope_exit& lock, std::optional kind = std::nullopt); + __requires_exclusive_lock_held(m_lock) std::shared_ptr StartTransition(TransitionKind kind, ContainerEvent expectedEvent); + + // Returns with both locks held when no transition is active or the active transition matches kind. + template + void WaitForConflictingTransitionToComplete( + wil::rwlock_release_exclusive_scope_exit& lock, TLifecycleLock& lifecycleLock, std::optional kind = std::nullopt); + void WaitForTransitionCompletion(const std::shared_ptr& transition) const; void AttachToTransition(const std::shared_ptr& transition) const; @@ -211,6 +216,8 @@ class WSLCContainerImpl : public std::enable_shared_from_this __requires_shared_lock_held(m_lock) std::string InspectLockHeld() const; + // Stop request setup holds this shared; state-changing operations and event delivery hold it exclusively. + wil::srwlock m_lifecycleGate; mutable wil::srwlock m_lock; std::string m_name; std::string m_image; diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 353bd61d50..2c9aed0cb8 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -6586,11 +6586,7 @@ class WSLCTests std::thread stopThread([&]() { VERIFY_SUCCEEDED(container.Get().Stop(WSLCSignalNone, -1)); }); auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { - // TODO: calling Kill() here hangs since Stop() holds the container lock. - // Update this once fixed to: - // LOG_IF_FAILED(container.Get().Kill(WSLCSignalSIGKILL)); - - LOG_IF_FAILED(initProcess.Get().Signal(WSLCSignalSIGKILL)); + LOG_IF_FAILED(container.Get().Kill(WSLCSignalSIGKILL)); if (stopThread.joinable()) { @@ -6866,7 +6862,7 @@ class WSLCTests output.ExpectConsume("stopping\n"); - // Kill fails after the container exits, so success proves it joined the active Stop transition. + // A second lifecycle request must reach Docker while the indefinite Stop request is blocked. wil::unique_event killStarted{wil::EventOptions::ManualReset}; killThread = std::thread([&]() { killStarted.SetEvent(); @@ -6874,7 +6870,10 @@ class WSLCTests }); VERIFY_IS_TRUE(killStarted.wait(30 * 1000)); - VERIFY_ARE_EQUAL(WaitForSingleObject(killThread.native_handle(), 100), WAIT_TIMEOUT); + VERIFY_ARE_EQUAL(WaitForSingleObject(killThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + killThread.join(); + VERIFY_SUCCEEDED(killResult); + VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 100), WAIT_TIMEOUT); const char stopInput = '\n'; DWORD bytesWritten{}; @@ -6883,14 +6882,67 @@ class WSLCTests input.reset(); VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); - VERIFY_ARE_EQUAL(WaitForSingleObject(killThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); stopThread.join(); - killThread.join(); cleanup.release(); VERIFY_SUCCEEDED(stopResult); - VERIFY_SUCCEEDED(killResult); + VERIFY_ARE_EQUAL(container.State(), WslcContainerStateExited); + } + + WSLC_TEST_METHOD(ConcurrentContainerStopTimeoutOverride) + { + WSLCContainerLauncher launcher( + "debian:latest", + "test-concurrent-stop-timeout", + {"/bin/sh", "-c", "trap 'echo stopping; read value; exit 0' TERM; echo ready; while true; do sleep 1; done"}, + {}, + "host", + WSLCProcessFlagsStdin); + + auto container = launcher.Launch(*m_defaultSession); + auto initProcess = container.GetInitProcess(); + auto input = initProcess.GetStdHandle(0); + auto outputHandle = initProcess.GetStdHandle(1); + PartialHandleRead output{outputHandle.get()}; + output.ExpectConsume("ready\n"); + + HRESULT indefiniteStopResult{}; + HRESULT immediateStopResult{}; + std::thread indefiniteStopThread; + std::thread immediateStopThread; + + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { + input.reset(); + + if (indefiniteStopThread.joinable()) + { + indefiniteStopThread.join(); + } + + if (immediateStopThread.joinable()) + { + immediateStopThread.join(); + } + }); + + indefiniteStopThread = + std::thread([&]() { indefiniteStopResult = container.Get().Stop(WSLCSignalNone, WSLC_STOP_TIMEOUT_NONE); }); + + output.ExpectConsume("stopping\n"); + VERIFY_ARE_EQUAL(WaitForSingleObject(indefiniteStopThread.native_handle(), 100), WAIT_TIMEOUT); + + immediateStopThread = std::thread([&]() { immediateStopResult = container.Get().Stop(WSLCSignalNone, 0); }); + + VERIFY_ARE_EQUAL(WaitForSingleObject(immediateStopThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + VERIFY_ARE_EQUAL(WaitForSingleObject(indefiniteStopThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + + indefiniteStopThread.join(); + immediateStopThread.join(); + cleanup.release(); + + VERIFY_SUCCEEDED(indefiniteStopResult); + VERIFY_SUCCEEDED(immediateStopResult); VERIFY_ARE_EQUAL(container.State(), WslcContainerStateExited); } From 4b9e7f1c1eccaf50cca70765ac15c9f56751ca66 Mon Sep 17 00:00:00 2001 From: kvega005 Date: Fri, 31 Jul 2026 11:14:01 -0700 Subject: [PATCH 15/15] Fix concurrent start/stop/delete calls --- src/windows/wslcsession/WSLCContainer.cpp | 22 +--- src/windows/wslcsession/WSLCContainer.h | 9 +- test/windows/WSLCTests.cpp | 136 +++++++++++++++++++--- 3 files changed, 129 insertions(+), 38 deletions(-) diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index ed5ce2c976..90d4628bb9 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -743,7 +743,7 @@ void WSLCContainerImpl::Attach(LPCSTR DetachKeys, WSLCHandle* Stdin, WSLCHandle* void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessStartOptions* StartOptions) { std::shared_ptr transition; - auto lifecycleLock = m_lifecycleGate.lock_exclusive(); + auto lifecycleLock = m_lifecycleLock.lock_shared(); auto lock = m_lock.lock_exclusive(); WaitForConflictingTransitionToComplete(lock, lifecycleLock); @@ -886,9 +886,8 @@ void WSLCContainerImpl::Start(WSLCContainerStartFlags Flags, const WSLCProcessSt AttachToTransition(transition); } -template void WSLCContainerImpl::WaitForConflictingTransitionToComplete( - wil::rwlock_release_exclusive_scope_exit& lock, TLifecycleLock& lifecycleLock, std::optional kind) + wil::rwlock_release_exclusive_scope_exit& lock, wil::rwlock_release_shared_scope_exit& lifecycleLock, std::optional kind) { while (m_transition && (!kind.has_value() || m_transition->Kind != kind.value())) { @@ -899,16 +898,7 @@ void WSLCContainerImpl::WaitForConflictingTransitionToComplete( WaitForTransitionCompletion(transition); } - if constexpr (std::is_same_v) - { - lifecycleLock = m_lifecycleGate.lock_shared(); - } - else - { - static_assert(std::is_same_v); - lifecycleLock = m_lifecycleGate.lock_exclusive(); - } - + lifecycleLock = m_lifecycleLock.lock_shared(); lock = m_lock.lock_exclusive(); } } @@ -964,7 +954,7 @@ void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional exitCod std::shared_ptr transition; { - auto lifecycleLock = m_lifecycleGate.lock_exclusive(); + auto lifecycleLock = m_lifecycleLock.lock_exclusive(); auto lock = m_lock.lock_exclusive(); transition = m_transition; @@ -1023,7 +1013,7 @@ void WSLCContainerImpl::Stop(WSLCSignal Signal, LONG TimeoutSeconds, bool Kill) std::shared_ptr transition; { - auto lifecycleLock = m_lifecycleGate.lock_shared(); + auto lifecycleLock = m_lifecycleLock.lock_shared(); auto lock = m_lock.lock_exclusive(); WaitForConflictingTransitionToComplete(lock, lifecycleLock, TransitionKind::Stop); @@ -1186,7 +1176,7 @@ __requires_exclusive_lock_held(m_lock) void WSLCContainerImpl::OnStopped(int exi void WSLCContainerImpl::Delete(WSLCDeleteFlags Flags) { std::shared_ptr transition; - auto lifecycleLock = m_lifecycleGate.lock_exclusive(); + auto lifecycleLock = m_lifecycleLock.lock_shared(); auto lock = m_lock.lock_exclusive(); WaitForConflictingTransitionToComplete(lock, lifecycleLock); diff --git a/src/windows/wslcsession/WSLCContainer.h b/src/windows/wslcsession/WSLCContainer.h index c1514fe958..f123df5dd0 100644 --- a/src/windows/wslcsession/WSLCContainer.h +++ b/src/windows/wslcsession/WSLCContainer.h @@ -187,9 +187,10 @@ class WSLCContainerImpl : public std::enable_shared_from_this __requires_exclusive_lock_held(m_lock) std::shared_ptr StartTransition(TransitionKind kind, ContainerEvent expectedEvent); // Returns with both locks held when no transition is active or the active transition matches kind. - template void WaitForConflictingTransitionToComplete( - wil::rwlock_release_exclusive_scope_exit& lock, TLifecycleLock& lifecycleLock, std::optional kind = std::nullopt); + wil::rwlock_release_exclusive_scope_exit& lock, + wil::rwlock_release_shared_scope_exit& lifecycleLock, + std::optional kind = std::nullopt); void WaitForTransitionCompletion(const std::shared_ptr& transition) const; void AttachToTransition(const std::shared_ptr& transition) const; @@ -216,8 +217,8 @@ class WSLCContainerImpl : public std::enable_shared_from_this __requires_shared_lock_held(m_lock) std::string InspectLockHeld() const; - // Stop request setup holds this shared; state-changing operations and event delivery hold it exclusively. - wil::srwlock m_lifecycleGate; + // Lifecycle requests hold this shared until their transitions are published; event delivery holds it exclusively. + wil::srwlock m_lifecycleLock; mutable wil::srwlock m_lock; std::string m_name; std::string m_image; diff --git a/test/windows/WSLCTests.cpp b/test/windows/WSLCTests.cpp index 2c9aed0cb8..55324c27e3 100644 --- a/test/windows/WSLCTests.cpp +++ b/test/windows/WSLCTests.cpp @@ -173,6 +173,19 @@ class WSLCTests return RunningWSLCContainer(std::move(rawContainer), {}); } + RunningWSLCContainer LaunchContainerWithBlockingStopHandler(const std::string& name) + { + WSLCContainerLauncher launcher( + "debian:latest", + name, + {"/bin/sh", "-c", "trap 'echo stopping; read value; exit 0' TERM; echo ready; while true; do sleep 1; done"}, + {}, + "host", + WSLCProcessFlagsStdin); + + return launcher.Launch(*m_defaultSession); + } + struct ListContainersResult { wil::unique_cotaskmem_array_ptr Containers; @@ -6824,15 +6837,7 @@ class WSLCTests WSLC_TEST_METHOD(ConcurrentContainerStopAndKill) { - WSLCContainerLauncher launcher( - "debian:latest", - "test-concurrent-container-stops", - {"/bin/sh", "-c", "trap 'echo stopping; read value; exit 0' TERM; echo ready; while true; do sleep 1; done"}, - {}, - "host", - WSLCProcessFlagsStdin); - - auto container = launcher.Launch(*m_defaultSession); + auto container = LaunchContainerWithBlockingStopHandler("test-concurrent-container-stops"); auto initProcess = container.GetInitProcess(); auto input = initProcess.GetStdHandle(0); auto outputHandle = initProcess.GetStdHandle(1); @@ -6892,15 +6897,7 @@ class WSLCTests WSLC_TEST_METHOD(ConcurrentContainerStopTimeoutOverride) { - WSLCContainerLauncher launcher( - "debian:latest", - "test-concurrent-stop-timeout", - {"/bin/sh", "-c", "trap 'echo stopping; read value; exit 0' TERM; echo ready; while true; do sleep 1; done"}, - {}, - "host", - WSLCProcessFlagsStdin); - - auto container = launcher.Launch(*m_defaultSession); + auto container = LaunchContainerWithBlockingStopHandler("test-concurrent-stop-timeout"); auto initProcess = container.GetInitProcess(); auto input = initProcess.GetStdHandle(0); auto outputHandle = initProcess.GetStdHandle(1); @@ -6946,6 +6943,109 @@ class WSLCTests VERIFY_ARE_EQUAL(container.State(), WslcContainerStateExited); } + WSLC_TEST_METHOD(ConcurrentContainerStopAndStart) + { + auto container = LaunchContainerWithBlockingStopHandler("test-concurrent-stop-start"); + auto initProcess = container.GetInitProcess(); + auto input = initProcess.GetStdHandle(0); + auto outputHandle = initProcess.GetStdHandle(1); + PartialHandleRead output{outputHandle.get()}; + output.ExpectConsume("ready\n"); + + HRESULT stopResult{}; + HRESULT startResult{}; + std::thread stopThread; + std::thread startThread; + + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { + input.reset(); + + if (stopThread.joinable()) + { + stopThread.join(); + } + + if (startThread.joinable()) + { + startThread.join(); + } + }); + + stopThread = std::thread([&]() { stopResult = container.Get().Stop(WSLCSignalNone, WSLC_STOP_TIMEOUT_NONE); }); + + output.ExpectConsume("stopping\n"); + VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 100), WAIT_TIMEOUT); + + startThread = std::thread([&]() { startResult = container.Get().Start(WSLCContainerStartFlagsNone, nullptr, nullptr); }); + + VERIFY_ARE_EQUAL(WaitForSingleObject(startThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + startThread.join(); + VERIFY_ARE_EQUAL(startResult, WSLC_E_CONTAINER_IS_RUNNING); + VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 100), WAIT_TIMEOUT); + + const char stopInput = '\n'; + DWORD bytesWritten{}; + VERIFY_WIN32_BOOL_SUCCEEDED(WriteFile(input.get(), &stopInput, sizeof(stopInput), &bytesWritten, nullptr)); + VERIFY_ARE_EQUAL(bytesWritten, static_cast(sizeof(stopInput))); + input.reset(); + + VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + stopThread.join(); + cleanup.release(); + + VERIFY_SUCCEEDED(stopResult); + VERIFY_ARE_EQUAL(container.State(), WslcContainerStateExited); + } + + WSLC_TEST_METHOD(ConcurrentContainerStopAndForceDelete) + { + auto container = LaunchContainerWithBlockingStopHandler("test-concurrent-stop-delete"); + auto initProcess = container.GetInitProcess(); + auto input = initProcess.GetStdHandle(0); + auto outputHandle = initProcess.GetStdHandle(1); + PartialHandleRead output{outputHandle.get()}; + output.ExpectConsume("ready\n"); + + HRESULT stopResult{}; + HRESULT deleteResult{}; + std::thread stopThread; + std::thread deleteThread; + + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { + input.reset(); + + if (stopThread.joinable()) + { + stopThread.join(); + } + + if (deleteThread.joinable()) + { + deleteThread.join(); + } + }); + + stopThread = std::thread([&]() { stopResult = container.Get().Stop(WSLCSignalNone, WSLC_STOP_TIMEOUT_NONE); }); + + output.ExpectConsume("stopping\n"); + VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 100), WAIT_TIMEOUT); + + deleteThread = std::thread([&]() { deleteResult = container.Get().Delete(WSLCDeleteFlagsForce); }); + + VERIFY_ARE_EQUAL(WaitForSingleObject(deleteThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + VERIFY_ARE_EQUAL(WaitForSingleObject(stopThread.native_handle(), 30 * 1000), WAIT_OBJECT_0); + + deleteThread.join(); + stopThread.join(); + input.reset(); + cleanup.release(); + + VERIFY_SUCCEEDED(stopResult); + VERIFY_SUCCEEDED(deleteResult); + container.SetDeleteOnClose(false); + VERIFY_ARE_EQUAL(container.State(), WslcContainerStateDeleted); + } + WSLC_TEST_METHOD(ForceDeleteAutoRemoveContainer) { WSLCContainerLauncher launcher("debian:latest", "test-force-delete-auto-remove", {"sleep", "99999"});