Skip to content

Commit 26e6c8d

Browse files
branchseerclaude
andcommitted
fix: handle tasks that close stdio without exiting
Add cancellation-aware wait after pipe reads in spawn_with_tracking. If a child closes stdout/stderr but stays alive (e.g., daemonizes), the pipe reads EOF but child.wait() would block forever without cancellation support. Add --daemonize flag to barrier test tool and e2e test verifying that daemonized concurrent tasks are properly killed on failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a7a0595 commit 26e6c8d

File tree

6 files changed

+47
-5
lines changed

6 files changed

+47
-5
lines changed

crates/vite_task/src/session/execute/spawn.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,12 @@ pub async fn spawn_with_tracking(
208208
}
209209

210210
// Wait for process termination and collect results.
211+
// The child may have closed its pipes without exiting (e.g., daemonized),
212+
// so we still need a cancellation arm here.
211213
match child_wait {
212214
ChildWait::Fspy(tracked_child) => {
215+
// fspy's wait_handle already monitors the cancellation token internally,
216+
// so no additional select! is needed here.
213217
let termination = tracked_child.wait_handle.await?;
214218
let duration = start.elapsed();
215219

@@ -285,7 +289,13 @@ pub async fn spawn_with_tracking(
285289
Ok(SpawnResult { exit_status: termination.status, duration })
286290
}
287291
ChildWait::Tokio(mut child) => {
288-
let exit_status = child.wait().await?;
292+
let exit_status = tokio::select! {
293+
status = child.wait() => status?,
294+
() = cancellation_token.cancelled() => {
295+
child.start_kill()?;
296+
child.wait().await?
297+
}
298+
};
289299
Ok(SpawnResult { exit_status, duration: start.elapsed() })
290300
}
291301
}

crates/vite_task_bin/tests/e2e_snapshots/fixtures/concurrent-execution/packages/a/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"name": "@concurrent/a",
33
"scripts": {
44
"build": "barrier ../../.barrier sync 2",
5-
"test": "barrier ../../.barrier test-sync 2 --exit=1"
5+
"test": "barrier ../../.barrier test-sync 2 --exit=1",
6+
"daemon": "barrier ../../.barrier daemon-sync 2 --exit=1"
67
}
78
}

crates/vite_task_bin/tests/e2e_snapshots/fixtures/concurrent-execution/packages/b/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"name": "@concurrent/b",
33
"scripts": {
44
"build": "barrier ../../.barrier sync 2",
5-
"test": "barrier ../../.barrier test-sync 2 --hang"
5+
"test": "barrier ../../.barrier test-sync 2 --hang",
6+
"daemon": "barrier ../../.barrier daemon-sync 2 --daemonize"
67
}
78
}

crates/vite_task_bin/tests/e2e_snapshots/fixtures/concurrent-execution/snapshots.toml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,10 @@ steps = ["vt run -r test"]
2020
[[e2e]]
2121
name = "failure kills concurrent cached tasks"
2222
steps = ["vt run -r --cache test"]
23+
24+
# Task b closes stdout/stderr after the barrier but stays alive (daemonizes).
25+
# The pipe reads EOF but the process doesn't exit. The runner must still be
26+
# able to kill it via the cancellation token + Job Object.
27+
[[e2e]]
28+
name = "failure kills daemonized concurrent tasks"
29+
steps = ["vt run -r --cache daemon"]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
source: crates/vite_task_bin/tests/e2e_snapshots/main.rs
3+
expression: e2e_outputs
4+
---
5+
[1]> vt run -r --cache daemon
6+
~/packages/a$ barrier ../../.barrier daemon-sync 2 --exit=1
7+
~/packages/b$ barrier ../../.barrier daemon-sync 2 --daemonize
8+
9+
10+
---
11+
vt run: 0/2 cache hit (0%), 2 failed. (Run `vt run --last-details` for full details)

packages/tools/src/barrier.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
#!/usr/bin/env node
22

3-
// barrier <dir> <prefix> <count> [--exit=<code>] [--hang]
3+
// barrier <dir> <prefix> <count> [--exit=<code>] [--hang] [--daemonize]
44
//
55
// Cross-platform concurrency barrier for testing.
66
// Creates <dir>/<prefix>_<pid>, then waits (via fs.watch) for <count> files
77
// matching <prefix>_* to exist in <dir>.
88
//
99
// Options:
1010
// --exit=<code> Exit with the given code after the barrier is met.
11-
// --hang Block on stdin after the barrier is met (for kill tests).
11+
// --hang Keep process alive after the barrier (for kill tests).
12+
// --daemonize Close stdout/stderr but keep process alive (for daemon kill tests).
1213
//
1314
// If tasks run concurrently, all participants arrive and the barrier resolves.
1415
// If tasks run sequentially, the first participant waits forever (test timeout).
@@ -19,12 +20,15 @@ import path from 'node:path';
1920
const positional = [];
2021
let exitCode = 0;
2122
let hang = false;
23+
let daemonize = false;
2224

2325
for (const arg of process.argv.slice(2)) {
2426
if (arg.startsWith('--exit=')) {
2527
exitCode = parseInt(arg.slice(7), 10);
2628
} else if (arg === '--hang') {
2729
hang = true;
30+
} else if (arg === '--daemonize') {
31+
daemonize = true;
2832
} else {
2933
positional.push(arg);
3034
}
@@ -44,6 +48,14 @@ function countMatches() {
4448
}
4549

4650
function onBarrierMet() {
51+
if (daemonize) {
52+
// Close stdout/stderr but keep the process alive. Simulates a daemon that
53+
// detaches from stdio — tests that the runner can still kill such processes.
54+
process.stdout.end();
55+
process.stderr.end();
56+
setInterval(() => {}, 1 << 30);
57+
return;
58+
}
4759
if (hang) {
4860
// Keep the process alive indefinitely — killed via signal when the runner cancels.
4961
// Use setInterval rather than stdin.resume() for cross-platform reliability.

0 commit comments

Comments
 (0)