Repository navigation
Natural EOF registered for piped input with non-interactive output - #1736
Draft
Dom Giandinoto (daamenik) wants to merge 1 commit into
Draft
Dom Giandinoto (daamenik) wants to merge 1 commit into
Dom Giandinoto (daamenik) wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Missing IsolationSession regressions leave the redirected-stdout hang unverified, while the documentation omits the unresolved Windows EOF limitation.
2 open issues
What changed in this PR
Addresses the MXC-side gap in #1678 by requesting IsolationSession stdin closure when pipe or file input ends and stdout is not a console.
Changes:
- Distinguishes natural EOF from cancellation and invokes the close callback after releasing the relay’s writer.
- Coordinates stdin closure with cleanup and adds diagnostic logging and unit tests.
- Documents the behavior for create-and-run and relayed lifecycle execution.
| File | Description |
|---|---|
src/mxc-sdk/src/core/mxc_common/process_util.rs |
Exposes reader cancellation state to IsolationSession. |
src/mxc-sdk/src/backends/isolation_session/common/pipe_relay.rs |
Adds EOF callbacks and tests for closure ordering, errors, cancellation, and panics. |
src/mxc-sdk/src/backends/isolation_session/common/manager.rs |
Connects EOF to platform stdin closure with synchronization and diagnostics. |
docs/development/architecture/backends/isolation-session/state-aware-rust.md |
Describes EOF handling for relayed lifecycle execution. |
docs/development/architecture/backends/isolation-session/oneshot.md |
Documents EOF closure, diagnostics, and cleanup timing. |
Review effort: Balanced
| owned_thread::call(&impersonation, || { | ||
| let _close = | ||
| close_lock.lock().unwrap_or_else(PoisonError::into_inner); | ||
| process.CloseStandardInput().map_err(|error| { |
Comment on lines
+166
to
+169
| When stdin is a pipe or file and stdout is not a console, MXC closes the | ||
| workload's stdin when the input ends, so programs such as `sort.exe` can | ||
| finish instead of timing out. Console input and ConPTY behavior are unchanged. | ||
| Errors reported by this close operation go to configured diagnostic logs. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
🔗 References
Fixes #1678
🔍 Validation
✅ Checklist
feature-acceptedCargo.lock, thedependency-feed-checkcheck passes (see pull request builds)📋 Issue Type
Microsoft Reviewers: Open in CodeFlow