Skip to content

Container lifecycle transitions#41140

Open
kvega005 wants to merge 32 commits into
microsoft:masterfrom
kvega005:user/kevinve/container-lifecycle
Open

Container lifecycle transitions#41140
kvega005 wants to merge 32 commits into
microsoft:masterfrom
kvega005:user/kevinve/container-lifecycle

Conversation

@kvega005

@kvega005 kvega005 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Updates WSLCContainerImpl so requested COM lifecycle operations are represented by an active StateTransition and completed by the corresponding Docker event. OnEvent() becomes the source of truth for committing container state and releasing lifecycle resources, while concurrent Stop/Kill callers share the same in-flight transition

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Problem. Container lifecycle requests and observed Docker state were previously handled through separate paths. Start() committed the Running state from the request path, Stop() coordinated with m_stopLock and m_stopNotification, and Delete() committed the Deleted state before separately waiting for m_destroyEvent. This split state and resource management between COM callers and Docker event callbacks, while concurrent Stop/Kill callers could not share the result of the same requested transition.

Change. Introduces StateTransition to represent an active state change requested through COM. A transition records the expected Docker event, a manual-reset completion event, any resulting exception, and deferred COM-wrapper cleanup. Lifecycle methods submit the Docker request while holding m_lock, publish the transition, release m_lock, and wait for OnEvent() to complete it.

OnEvent() now commits state based on the Docker event stream:

  • A Start event commits WslcContainerStateRunning and completes the matching Start transition.
  • A Stop event records the exit code, notifies plugins, releases processes and runtime resources, commits WslcContainerStateExited, and completes the matching Stop transition.
  • For auto-remove containers, Stop event handling submits the delete request and updates the active transition to expect Destroy, allowing the original Stop caller to wait through deletion.
  • A Destroy event commits WslcContainerStateDeleted, releases the remaining resources, disconnects the COM wrapper outside the container lock, and completes the matching Delete or auto-remove transition.
  • Unexpected or duplicate events do not overwrite a newer committed state.

m_transitionLock serializes lifecycle operations initiated through COM. Start and Delete hold it exclusively, while Stop/Kill hold it shared so concurrent callers can attach to the same active Stop transition. m_lock protects the active transition and container state, while waits occur without holding m_lock so OnEvent() can process the corresponding Docker event.

This replaces the dedicated Stop and Destroy notification objects and their fixed 60-second waits with one transition-completion mechanism.

Validation Steps Performed

  • Added WSLCTests::ConcurrentContainerStopAndKill to verify that a concurrent Kill joins an active Stop transition instead of issuing a competing request after the container exits.
  • Added WSLCTests::ForceDeleteAutoRemoveContainer to verify that force-deleting a running auto-remove container waits for Docker destruction, commits Deleted, disconnects the COM wrapper, and removes the container from lookup.
  • Updated PluginTests::WslcPullImageNotification to account for the container-stopping notification produced during event-driven session cleanup.

Copilot AI review requested due to automatic review settings July 22, 2026 18:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors WSLC container lifecycle handling to coordinate Start/Stop/Delete operations via a shared “transition” object, with event-driven completion, and renames the internal state update helper from Transition to CommitState.

Changes:

  • Introduce a StateTransition model (with completion event/exception propagation) plus m_transitionLock/m_transition to coordinate lifecycle operations and allow concurrent Stop() callers to join an in-flight transition.
  • Refactor lifecycle flow so Start(), Stop(), and Delete() publish a transition and wait for corresponding Docker events to complete it.
  • Extend Docker event tracking to include the "restart" action (ContainerEvent::Restart).

Reviewed changes

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

File Description
src/windows/wslcsession/WSLCContainer.h Adds transition coordination primitives and renames TransitionCommitState; updates lifecycle method signatures.
src/windows/wslcsession/WSLCContainer.cpp Implements transition creation/wait/completion and refactors event handling and Stop/Delete behavior around transitions.
src/windows/wslcsession/DockerEventTracker.h Adds ContainerEvent::Restart.
src/windows/wslcsession/DockerEventTracker.cpp Maps Docker "restart" events to ContainerEvent::Restart.

Comment thread src/windows/wslcsession/WSLCContainer.cpp Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

src/windows/wslcsession/WSLCContainer.cpp:920

  • OnEvent() is declared noexcept but it still contains throwing code paths (e.g., THROW_HR_IF in the Stop branch) and calls OnStopped()/ReleaseRuntimeResources() which can also throw. Since DockerEventTracker invokes callbacks without catching exceptions, this can either terminate the process (noexcept) or unwind out of the event dispatcher and leave m_transition uncleared, deadlocking Start/Stop/Delete waiters.
void WSLCContainerImpl::OnEvent(ContainerEvent event, std::optional<int> 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;

Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment thread src/windows/wslcsession/WSLCContainer.cpp Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 18:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (2)

src/windows/wslcsession/WSLCContainer.cpp:941

  • WSLCContainerImpl::OnEvent is declared noexcept, but this Stop branch uses THROW_HR_IF, which will throw on malformed events and immediately std::terminate due to noexcept. DockerEventTracker also invokes callbacks without a try/catch, so any exception escaping here can take down the event tracking thread/process. Please make the Stop branch non-throwing and complete any in-flight transition with an exception so waiters don't hang.
    else if (event == ContainerEvent::Stop)
    {
        THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value());
        OnStopped(exitCode.value(), eventTime);
    }

src/windows/wslcsession/WSLCContainer.cpp:1138

  • This method reacquires m_lock at the end (lock = m_lock.lock_exclusive()) but does not use it afterward. This looks like leftover code and adds unnecessary locking/noise; it can be removed.
    lock = m_lock.lock_exclusive();
}

Comment thread src/windows/wslcsession/DockerEventTracker.cpp
Copilot AI review requested due to automatic review settings July 22, 2026 19:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

src/windows/wslcsession/WSLCContainer.cpp:940

  • OnEvent() is declared noexcept, but the Stop branch uses THROW_HR_IF when exitCode is missing. Any throw inside a noexcept function will terminate the process. Also, if this occurs while a Stop() transition is waiting, the transition would never be completed, causing a hang.
    else if (event == ContainerEvent::Stop)
    {
        THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value());
        OnStopped(exitCode.value(), eventTime);

Comment thread src/windows/wslcsession/WSLCContainer.cpp
Copilot AI review requested due to automatic review settings July 22, 2026 19:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

src/windows/wslcsession/WSLCContainer.cpp:941

  • WSLCContainerImpl::OnEvent() is declared noexcept, but the Stop-path uses THROW_HR_IF(...). Any exception escaping a noexcept function will call std::terminate(), and DockerEventTracker invokes callbacks without a try/catch, so this would turn a malformed/partial Docker event (missing exitCode) into a process crash. Handle the missing exitCode without throwing (and still drive OnStopped so any waiting transition can complete).
    else if (event == ContainerEvent::Stop)
    {
        THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value());
        OnStopped(exitCode.value(), eventTime);
    }

Copilot AI review requested due to automatic review settings July 22, 2026 22:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

src/windows/wslcsession/WSLCContainer.cpp:941

  • WSLCContainerImpl::OnEvent() is declared noexcept but still uses THROW_HR_IF when handling ContainerEvent::Stop. If Docker sends a "die" event without an exitCode attribute, this will throw inside a noexcept callback (invoked by DockerEventTracker without try/catch), causing std::terminate and potentially taking down the event thread/process. Handle missing exitCode without throwing (log and use a default exit code, or ignore the event safely).
    else if (event == ContainerEvent::Stop)
    {
        THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value());
        OnStopped(exitCode.value(), eventTime);
    }

Copilot AI review requested due to automatic review settings July 22, 2026 23:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

src/windows/wslcsession/WSLCContainer.cpp:941

  • WSLCContainerImpl::OnEvent() is declared noexcept and is invoked from DockerEventTracker without any try/catch around the callback. Using THROW_HR_IF here will call std::terminate on a missing exitCode (or unwind out of the callback if noexcept is removed), potentially crashing the session/event thread and leaving waiters blocked. Handle the missing exitCode without throwing and complete any in-flight Stop transition with an error so WaitForTransition() doesn’t hang forever.
    else if (event == ContainerEvent::Stop)
    {
        THROW_HR_IF(E_UNEXPECTED, !exitCode.has_value());
        OnStopped(exitCode.value(), eventTime);
    }

@kvega005
kvega005 marked this pull request as ready for review July 23, 2026 16:27
@kvega005
kvega005 requested review from a team as code owners July 23, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants