Skip to content

[WSLC] Always warn when a stdin flood may have lost a command's output - #1620

Merged
Soham Das (SohamDas2021) merged 1 commit into
mainfrom
sohamdas2021-wslc-stdin-flood-flake
Oct 30, 2026
Merged

Soham Das (SohamDas2021) merged 1 commit into
mainfrom
sohamdas2021-wslc-stdin-flood-flake

Conversation

@SohamDas2021

@SohamDas2021 Soham Das (SohamDas2021) commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📖 Description

When a WSLc command exits without reading a large amount of its stdin, the WSLc SDK sometimes drops the command's output and reports its exit 30 s or more late. MXC warned only when that report took longer than its own 30 s wait, so when the SDK reported exactly 30 s late, a few milliseconds decided whether the warning appeared. On some hosts it didn't, and output was lost with no warning.

  • Warning: now appears whenever the SDK reports the exit 10 s or more after the command ended. A normal exit is reported within milliseconds. This covers both the CLI and the SDK streaming path.
  • Test: the flood case runs sleep 40 instead of sleep 3. A command that ends that long after its input backs up is reported exactly 30 s late, the delay the old check could miss.
  • Docs: the exit is reported "30 seconds or more" late, not "about 50 seconds".

MXC cannot stop the SDK from dropping the output; it can only warn that this may have happened. No spec is needed: this is a bug fix with no change to the API or schema.

🔗 References

Fixes #1605. Follow-up to #1564. This edits the same part of wsl_container_runner.rs as #1597, so whichever merges second needs a small rebase.

🔍 Validation

Windows with WSL 3.0.4.0 (the same version as the CI hosts) and WSLc SDK 3.0.3.

Command Result
cargo fmt --all -- --check Passed
cargo clippy -p mxc-sdk --features wslc --all-targets -- -D warnings Passed
cargo test -p mxc-sdk --features wslc --lib -- wslc Passed: 426 passed, 0 failed, 3 ignored
Flood case run alone through wxc-exec, before the fix (6 runs) 2 of 6 lost output with no warning, which reproduces #1605
Same, after the fix (6 runs) All 6 returned the output or the warning

Not run: the full run_wslc_all_tests.ps1 suite and the .NET and Node live WSLc tests.

✅ Checklist

  • Signed the Contributor License Agreement
  • Linked to an issue
  • For features or changes requiring a spec: linked the issue with maintainer-applied feature-accepted
  • For implementation: linked the required approved and merged design/API spec, or explained why no spec is required
  • Feature issue remains open unless this PR completes all agreed implementation, validation, and documentation
  • Updated documentation (if applicable)
  • Updated Copilot instructions (if build, architecture, or conventions changed)
  • If this PR changes Cargo.lock, the dependency-feed-check check passes (see pull request builds)

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 29, 2026 23:48
@SohamDas2021
Soham Das (SohamDas2021) requested a review from a team as a code owner October 29, 2026 23:48
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

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.

🟡 Changes recommended

The callback-delay check measures time spent in wait() rather than exit-to-callback latency, so it can miss late callbacks and associated output loss when callers drain output before waiting.

1 open issue
What changed in this PR

This follow-up to #1564 addresses intermittent missing warnings after WSLc stdin floods, reported in #1605, without changing public APIs or schemas.

Changes:

  • Uses a 10-second callback-wait threshold instead of requiring the 30-second wait to expire.
  • Extends the stdin-flood regression command to exercise the minimum observed late-exit delay.
  • Corrects documentation to describe exit reporting as delayed by 30 seconds or more.
File Description
tests/​scripts/​run_wslc_all_tests.ps1 Runs the flood command for 40 seconds with a 120-second execution timeout.
src/​mxc-sdk/​src/​backends/​wslc/​common/​wsl_container_runner.rs Adds elapsed-time detection for the potentially incomplete-output warning.
docs/​backends/​wslc/​wsl-container-getting-started.md Corrects the documented delay caused by unread stdin.

Review effort: Balanced

// below may leak the context, since the SDK can call back late.
let mut confirmed = io_ctx.wait_for_exit_callback();
let callback_late = !confirmed;
let callback_late = exit_seen.elapsed() >= LATE_EXIT_THRESHOLD;
@SohamDas2021
Soham Das (SohamDas2021) merged commit b4c1007 into main Oct 30, 2026
30 checks passed
@SohamDas2021
Soham Das (SohamDas2021) deleted the sohamdas2021-wslc-stdin-flood-flake branch October 30, 2026 01:15
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.

[WSLC] "Stdin Flood" Test Failing During Validation

3 participants