Automatically add process handle & session termination handle to synchronous IO#14239
Automatically add process handle & session termination handle to synchronous IO#14239OneBlue wants to merge 7 commits intofeature/wsl-for-appsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new helper method WSLASession::CreateIOContext() that automatically sets up cancellation handles for synchronous IO operations in WSLA sessions. The method creates a MultiHandleWait instance pre-configured with event handles that will cancel the operation if either the session terminates or the calling process exits.
Changes:
- Added
CreateIOContext()method that automatically adds session termination and client process exit handles to IO operations - Updated
ImportImageImpl,ExportContainerImpl, andSaveImageImplto use the new helper method - Made
MultiHandleWaitmovable to support returning it fromCreateIOContext() - Partially updated
PullImageto useCreateIOContext(), but the migration appears incomplete
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/windows/common/relay.hpp | Added move semantics (NON_COPYABLE and DEFAULT_MOVABLE macros) to MultiHandleWait class to enable returning it from CreateIOContext() |
| src/windows/wslasession/WSLASession.h | Declared new CreateIOContext() public method that returns a pre-configured MultiHandleWait instance |
| src/windows/wslasession/WSLASession.cpp | Implemented CreateIOContext() and migrated ImportImageImpl, ExportContainerImpl, and SaveImageImpl to use it; PullImage partially migrated; BuildImage intentionally not migrated due to in-progress changes |
|
|
||
| auto requestContext = m_dockerClient->BuildImage(ContentLength, dockerfilePath, tag); | ||
|
|
||
| // Not touching this since a change is in progress on BuildImage(). |
There was a problem hiding this comment.
This comment suggests BuildImage() is not being updated due to an in-progress change, but it would be helpful to add more context. Consider clarifying what change is in progress (e.g., a PR number or description) so future readers understand why this method wasn't updated to use CreateIOContext() like the other methods.
| // Not touching this since a change is in progress on BuildImage(). | |
| // NOTE: Other operations in this class use CreateIOContext() for setting up I/O handling. | |
| // BuildImage() is currently being refactored separately; once that work is complete, | |
| // update this path to use CreateIOContext() as well for consistency. |
| return riid == __uuidof(IWSLASession) ? S_OK : S_FALSE; | ||
| } | ||
|
|
||
| // TODO consider allowing callers to pass cancellation handles. |
There was a problem hiding this comment.
The TODO suggests allowing callers to pass custom cancellation handles. This would be a useful enhancement, but consider whether the current design is flexible enough. For example, if a caller needs different cancellation behavior (e.g., only cancel on session termination but not on client exit), they would have no way to achieve this with the current design. Consider adding an optional parameter to CreateIOContext() to allow customization of which handles to add.
…oneblue/exit-cancel
…s json" This reverts commit 441ccd8.
1 participant
Summary of the Pull Request
This change adds a method in WSLASession to create an IO operation that will automatically get cancelled if either the calling process terminates and the session termination event is signaled.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed