Fix spawnStreamAsync hang: end provided pipes on failure paths without truncating output

[bwateratmsft]

Summary

Fixes #540.

spawnStreamAsync only ended caller-provided stdOutPipe/stdErrPipe streams on the cancellation path. On a spawn error (e.g. ENOENT when the executable is not on PATH) or a pre-spawn throw (already-cancelled token, or Windows executable-resolution failure), the child's stdio streams never deliver EOF to the destination, so a caller awaiting an AccumulatorStream (which resolves only on the stream's finish event) could hang indefinitely. This was observed in Azure/azure-dev#9136, where an ENOENT turned a clean "not found" rejection into an indefinite hang.

Changes

• Added an idempotent endPipes() helper and invoke it on the two paths where the child streams never flow:
  • pre-spawn throw — already-cancelled token / executable-resolution failure (wrapped in try/catch so the pipes are ended before rethrowing)
  • spawn error event (e.g. ENOENT)
• Deliberately do NOT end the pipes on the exit path. The childProcess.stdout.pipe(dest) wiring already uses the default { end: true }, so the destination is ended when the source reaches EOF — after a full drain, race-free. This is what makes a non-zero exit settle the accumulator.
• Added an early return after the cancellation reject in the error/exit handlers, so a cancelled token no longer falls through to a redundant second settle (avoids tripping Node's multipleResolves diagnostics). Addresses Copilot code-review feedback.
• Cancellation behavior is otherwise unchanged.

Why not end on exit?

An earlier iteration also ended pipes in the exit handler. Testing with a slow/backpressured destination and a child that writes a large payload then exits non-zero showed the exit event fires while stdout is still draining, so a manual .end() truncated the tail and threw write after end. AccumulatorStream writes synchronously so it usually drains fast enough to hide this, but any slower caller-provided pipe (file, socket, transform) would lose data. Relying on the pipe's EOF-driven end avoids the race entirely while still ensuring the accumulator settles on non-zero exit.

Tests

Added (unit) regression tests (no Docker required; the default mocha run greps (unit)), each with timeout-guarded awaits so a re-hang fails deterministically:

• ENOENT spawn error → rejects and accumulators settle
• Non-zero exit → ChildProcessError and accumulators settle (stderr captured)
• Pre-spawn cancellation → CancellationError and accumulators settle
• Backpressure guard → a slow destination receives the full output with no stream error on non-zero exit (fails deterministically on the buggy end-on-exit, passes on the fix)

Verification

• All 10 unit tests pass; lint and build are clean.
• The backpressure regression test fails 3/3 against the buggy end-on-exit and passes 3/3 on the fix.

Changelog

Added a Fixed entry under the 1.0.0 - 21 July 2026 release.

[Copilot]

Pull request overview

This PR fixes a hang in spawnStreamAsync when callers provide stdOutPipe/stdErrPipe streams (notably AccumulatorStream) and the spawn fails before the child’s stdio can naturally reach EOF. It ensures those caller-provided pipes are ended on pre-spawn failure and on the child process error event, preventing downstream awaits on finish from hanging indefinitely.

Changes:

• Added an endPipes() helper and invoked it on pre-spawn failures and spawn error events (e.g. ENOENT) so caller-provided pipes always settle on these terminal paths.
• Added unit regression tests covering spawn ENOENT, pre-spawn cancellation, non-zero exit settling behavior, and a backpressure scenario to ensure output isn’t truncated on non-zero exit.
• Updated the vscode-processutils changelog with a 1.0.0 “Fixed” entry referencing #540.

Reviewed changes

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

File	Description
packages/vscode-processutils/src/utils/spawnStreamAsync.ts	Ends caller-provided pipes on pre-spawn and spawn-error terminal paths; documents why exit-path does not manually end pipes.
packages/vscode-processutils/src/test/spawnStreamAsync.test.ts	Adds unit tests to prevent regressions (hang prevention + backpressure/non-truncation guard).
packages/vscode-processutils/CHANGELOG.md	Documents the fix under 1.0.0.

Comments suppressed due to low confidence (2)

packages/vscode-processutils/src/utils/spawnStreamAsync.ts:186

• In the spawn error handler, when cancellationToken.isCancellationRequested is true you call reject(...) and then immediately fall through to reject(err). While the second reject is ignored, it can still trigger multipleResolves diagnostics and makes the control flow harder to reason about. Return after rejecting the cancellation error (or use an else).

            if (cancellationToken.isCancellationRequested) {
                reject(new CancellationError('Command cancelled', cancellationToken));
            }

            reject(err);

packages/vscode-processutils/src/utils/spawnStreamAsync.ts:200

• In the exit handler, if cancellationToken.isCancellationRequested is true you reject with a CancellationError but then continue on to resolve/reject based on code/signal. Add an early return after the cancellation rejection to avoid multiple settle attempts and keep the logic unambiguous.

            if (cancellationToken.isCancellationRequested) {
                reject(new CancellationError('Command cancelled', cancellationToken));
            }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.