Container lifecycle transitions#41140
Conversation
There was a problem hiding this comment.
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
StateTransitionmodel (with completion event/exception propagation) plusm_transitionLock/m_transitionto coordinate lifecycle operations and allow concurrentStop()callers to join an in-flight transition. - Refactor lifecycle flow so
Start(),Stop(), andDelete()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 Transition → CommitState; 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. |
There was a problem hiding this comment.
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;
There was a problem hiding this comment.
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();
}
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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 usesTHROW_HR_IF(...). Any exception escaping anoexceptfunction will callstd::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);
}
There was a problem hiding this comment.
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);
}
There was a problem hiding this comment.
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);
}
Summary of the Pull Request
Updates
WSLCContainerImplso requested COM lifecycle operations are represented by an activeStateTransitionand 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 transitionPR Checklist
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 withm_stopLockandm_stopNotification, andDelete()committed the Deleted state before separately waiting form_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
StateTransitionto 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 holdingm_lock, publish the transition, releasem_lock, and wait forOnEvent()to complete it.OnEvent()now commits state based on the Docker event stream:WslcContainerStateRunningand completes the matching Start transition.WslcContainerStateExited, and completes the matching Stop transition.Stopevent handling submits the delete request and updates the active transition to expect Destroy, allowing the original Stop caller to wait through deletion.WslcContainerStateDeleted, releases the remaining resources, disconnects the COM wrapper outside the container lock, and completes the matching Delete or auto-remove transition.m_transitionLockserializes 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_lockprotects the active transition and container state, while waits occur without holdingm_locksoOnEvent()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
WSLCTests::ConcurrentContainerStopAndKillto verify that a concurrent Kill joins an active Stop transition instead of issuing a competing request after the container exits.WSLCTests::ForceDeleteAutoRemoveContainerto 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.PluginTests::WslcPullImageNotificationto account for the container-stopping notification produced during event-driven session cleanup.