Skip to content

Commit 60e8a13

Browse files
test: share one cross-platform tree-kill helper across the dev-server suites
The three spawn-based suites (process-cleanup, metro-compiler, metro-startup-order) each carried their own killTree copy, and all three single-killed on POSIX — so a dev server's worker processes were orphaned on teardown (only Windows' taskkill /T walked the tree). Extract one killProcessTree into tests/test/tests/process-tree.ts: POSIX signals the whole process group (negative pid), Windows uses taskkill /F /T. Spawn the dev servers detached on POSIX so the group-kill reaches their workers — mirrors packages/test/src/setupTest.ts. Add cross-platform positive/negative tests that pin the contract: the tree-kill reaps the worker, while a bare single-pid kill leaves it orphaned (the reason the tree-kill exists). Windows uses a detached worker that escapes the job object; POSIX uses an in-group worker. Validated on Windows 11 (process-cleanup 3 passed, metro 2 passed, oxlint/tsc clean) and the POSIX group-kill mechanism on Linux (WSL).
1 parent 21a38e2 commit 60e8a13

4 files changed

Lines changed: 219 additions & 56 deletions

File tree

tests/test/tests/metro-compiler.test.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createRequire } from 'node:module'
33
import { dirname, join } from 'node:path'
44
import getPort from 'get-port'
55
import { describe, expect, it, afterEach } from 'vitest'
6+
import { killProcessTree } from './process-tree'
67

78
// Resolve one's JS entry (run.mjs) instead of node_modules/.bin/one: on Windows
89
// the .bin shim is a .cmd/.ps1/.exe wrapper that `node <path>` can't load
@@ -13,23 +14,6 @@ const oneRunEntry = join(
1314
'run.mjs'
1415
)
1516

16-
// node cannot signal a process tree on Windows; taskkill /T kills spawned workers too
17-
function killTree(proc: ChildProcess) {
18-
if (!proc.pid) return
19-
if (process.platform === 'win32') {
20-
try {
21-
spawn('taskkill', ['/F', '/T', '/PID', String(proc.pid)], { stdio: 'ignore' })
22-
} catch {}
23-
} else {
24-
proc.kill('SIGTERM')
25-
setTimeout(() => {
26-
try {
27-
proc.kill('SIGKILL')
28-
} catch {}
29-
}, 1000)
30-
}
31-
}
32-
3317
/**
3418
* Test that the React compiler works through the Metro bundling path.
3519
*
@@ -50,7 +34,7 @@ describe('Metro React compiler', { retry: 1 }, () => {
5034

5135
afterEach(() => {
5236
if (devServer) {
53-
killTree(devServer)
37+
killProcessTree(devServer.pid)
5438
devServer = null
5539
}
5640
})
@@ -69,6 +53,8 @@ describe('Metro React compiler', { retry: 1 }, () => {
6953
DEBUG: 'vxrn:*,vite-plugin-metro:*',
7054
},
7155
stdio: ['ignore', 'pipe', 'pipe'],
56+
// group leader on POSIX so killProcessTree can signal the whole group
57+
detached: process.platform !== 'win32',
7258
})
7359

7460
devServer.on('exit', (code) => {

tests/test/tests/metro-startup-order.test.ts

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import { spawn, type ChildProcess } from 'node:child_process'
1+
import { spawn } from 'node:child_process'
22
import { createRequire } from 'node:module'
33
import { dirname, join } from 'node:path'
44
import getPort from 'get-port'
55
import { describe, expect, it } from 'vitest'
6+
import { killProcessTree } from './process-tree'
67

78
// Resolve one's JS entry (run.mjs) instead of node_modules/.bin/one: on Windows
89
// the .bin shim is a .cmd/.ps1/.exe wrapper that `node <path>` can't load
@@ -13,23 +14,6 @@ const oneRunEntry = join(
1314
'run.mjs'
1415
)
1516

16-
// node cannot signal a process tree on Windows; taskkill /T kills spawned workers too
17-
function killTree(proc: ChildProcess) {
18-
if (!proc.pid) return
19-
if (process.platform === 'win32') {
20-
try {
21-
spawn('taskkill', ['/F', '/T', '/PID', String(proc.pid)], { stdio: 'ignore' })
22-
} catch {}
23-
} else {
24-
proc.kill('SIGTERM')
25-
setTimeout(() => {
26-
try {
27-
proc.kill('SIGKILL')
28-
} catch {}
29-
}, 1000)
30-
}
31-
}
32-
3317
/**
3418
* Test that Metro initialization happens AFTER Vite server is fully started.
3519
*
@@ -61,6 +45,8 @@ describe('Metro startup order', () => {
6145
DEBUG: 'vxrn:*',
6246
},
6347
stdio: ['ignore', 'pipe', 'pipe'],
48+
// group leader on POSIX so killProcessTree can signal the whole group
49+
detached: process.platform !== 'win32',
6450
})
6551

6652
const collectLogs = (data: Buffer) => {
@@ -104,8 +90,8 @@ describe('Metro startup order', () => {
10490
}, 100)
10591
})
10692

107-
// Kill the server (tree-kill on Windows; SIGTERM→SIGKILL on POSIX)
108-
killTree(devServer)
93+
// Kill the server (tree-kill: taskkill /T on Windows, process-group on POSIX)
94+
killProcessTree(devServer.pid)
10995
await new Promise((r) => setTimeout(r, 500))
11096

11197
// Verify the order - Vite should be ready BEFORE Metro

tests/test/tests/process-cleanup.test.ts

Lines changed: 154 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createRequire } from 'node:module'
33
import { dirname, join } from 'node:path'
44
import { afterEach, describe, expect, test } from 'vitest'
55
import getPort from 'get-port'
6+
import { killProcessTree } from './process-tree'
67

78
// Resolve one's JS entry (run.mjs) instead of node_modules/.bin/one: on Windows
89
// the .bin shim is a .cmd/.ps1/.exe wrapper that `node <path>` can't load
@@ -13,20 +14,6 @@ const oneRunEntry = join(
1314
'run.mjs'
1415
)
1516

16-
// node cannot signal a process tree on Windows; taskkill /T kills spawned workers too
17-
function killTree(proc: ChildProcess) {
18-
if (!proc.pid) return
19-
if (process.platform === 'win32') {
20-
try {
21-
spawn('taskkill', ['/F', '/T', '/PID', String(proc.pid)], { stdio: 'ignore' })
22-
} catch {}
23-
} else {
24-
try {
25-
process.kill(proc.pid, 'SIGKILL')
26-
} catch {}
27-
}
28-
}
29-
3017
// Helper to wait for a process to exit
3118
function waitForExit(proc: ChildProcess, timeout = 5000): Promise<number | null> {
3219
return new Promise((resolve) => {
@@ -55,10 +42,10 @@ describe('process cleanup', () => {
5542
afterEach(() => {
5643
// Clean up any leftover processes (tree-kill so dev-server workers don't orphan)
5744
if (devServer?.pid && isRunning(devServer.pid)) {
58-
killTree(devServer)
45+
killProcessTree(devServer.pid)
5946
}
6047
if (wrapper?.pid && isRunning(wrapper.pid)) {
61-
killTree(wrapper)
48+
killProcessTree(wrapper.pid)
6249
}
6350
})
6451

@@ -129,6 +116,8 @@ describe('process cleanup', () => {
129116
devServer = spawn('node', [oneRunEntry, 'dev', '--port', port.toString()], {
130117
cwd: process.cwd(),
131118
stdio: 'pipe',
119+
// group leader on POSIX so killProcessTree can signal the whole group
120+
detached: process.platform !== 'win32',
132121
})
133122

134123
// Wait for dev server to start
@@ -142,7 +131,154 @@ describe('process cleanup', () => {
142131
await new Promise((r) => setTimeout(r, 2000))
143132
expect(isRunning(devServer.pid!)).toBe(true)
144133

145-
// Clean up (tree-kill so dev-server workers don't orphan on Windows)
146-
killTree(devServer)
134+
// Clean up (tree-kill so dev-server workers don't orphan)
135+
killProcessTree(devServer.pid)
147136
}, 15000)
148137
})
138+
139+
// Pins the cross-platform tree-kill contract that killProcessTree (./process-tree,
140+
// and packages/test/src/setup.ts) relies on. The mechanisms differ by platform, so
141+
// each gets a positive (the whole tree is reaped) and a negative (a bare single-PID
142+
// kill leaves a worker orphaned — i.e. why a plain process.kill is not enough):
143+
// - Windows: process.kill(pid) is TerminateProcess and does not walk the tree; a
144+
// worker that broke away from libuv's job object (detached) survives it, while
145+
// taskkill /F /T walks the PID tree and reaps it.
146+
// - POSIX: process.kill(pid) signals only the leader; killProcessTree signals the
147+
// whole process group (negative pid), so the workers it forked go too.
148+
describe('process tree termination', () => {
149+
const spawnedPids: number[] = []
150+
151+
afterEach(() => {
152+
// safety net: force-kill anything a test spawned, even if it failed mid-way
153+
for (const pid of spawnedPids) {
154+
try {
155+
if (process.platform === 'win32') {
156+
spawn('taskkill', ['/F', '/T', '/PID', String(pid)], { stdio: 'ignore' })
157+
} else {
158+
try {
159+
process.kill(-pid, 'SIGKILL')
160+
} catch {
161+
// not a group leader
162+
}
163+
try {
164+
process.kill(pid, 'SIGKILL')
165+
} catch {
166+
// already gone
167+
}
168+
}
169+
} catch {
170+
// already gone
171+
}
172+
}
173+
spawnedPids.length = 0
174+
})
175+
176+
// Spawn a server -> worker tree, resolving once the worker PID is known. The
177+
// server is a group leader on POSIX (detached) so killProcessTree's group-kill
178+
// reaches the worker; on Windows detached is irrelevant to taskkill /T. The
179+
// worker is spawned detached only when we want it to escape the parent's job
180+
// object / process group (the case a single-process kill cannot clean up).
181+
function spawnTree(
182+
detachedWorker: boolean
183+
): Promise<{ server: ChildProcess; workerPid: number }> {
184+
const server = spawn(
185+
'node',
186+
[
187+
'-e',
188+
`
189+
const { spawn } = require('child_process')
190+
const worker = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], {
191+
detached: ${detachedWorker},
192+
stdio: 'ignore',
193+
})
194+
${detachedWorker ? 'worker.unref()' : ''}
195+
process.stdout.write('WORKER_PID=' + worker.pid + '\\n')
196+
setInterval(() => {}, 1000)
197+
`,
198+
],
199+
{ stdio: ['ignore', 'pipe', 'ignore'], detached: process.platform !== 'win32' }
200+
)
201+
return new Promise((resolve) => {
202+
let out = ''
203+
server.stdout?.on('data', (d) => {
204+
out += d.toString()
205+
const match = out.match(/WORKER_PID=(\d+)\r?\n/)
206+
if (match) {
207+
const workerPid = Number(match[1])
208+
if (server.pid) spawnedPids.push(server.pid)
209+
spawnedPids.push(workerPid)
210+
resolve({ server, workerPid })
211+
}
212+
})
213+
})
214+
}
215+
216+
test.skipIf(process.platform !== 'win32')(
217+
'killProcessTree reaps a detached worker that escaped the job object (Windows)',
218+
async () => {
219+
const { server, workerPid } = await spawnTree(true)
220+
expect(server.pid).toBeTruthy()
221+
expect(isRunning(server.pid!)).toBe(true)
222+
expect(isRunning(workerPid)).toBe(true)
223+
224+
killProcessTree(server.pid)
225+
await new Promise((r) => setTimeout(r, 2000))
226+
227+
expect(isRunning(server.pid!)).toBe(false)
228+
expect(isRunning(workerPid)).toBe(false)
229+
},
230+
15000
231+
)
232+
233+
test.skipIf(process.platform !== 'win32')(
234+
'a bare process.kill orphans a detached worker on Windows (why tree-kill is needed)',
235+
async () => {
236+
const { server, workerPid } = await spawnTree(true)
237+
expect(isRunning(workerPid)).toBe(true)
238+
239+
// TerminateProcess does not walk the tree, and the detached worker broke away
240+
// from the job object — so it survives as an orphan
241+
process.kill(server.pid!, 'SIGKILL')
242+
await new Promise((r) => setTimeout(r, 2000))
243+
244+
expect(isRunning(server.pid!)).toBe(false)
245+
expect(isRunning(workerPid)).toBe(true)
246+
// (afterEach reaps the orphaned worker)
247+
},
248+
15000
249+
)
250+
251+
test.skipIf(process.platform === 'win32')(
252+
'killProcessTree reaps an in-group worker (POSIX group-kill)',
253+
async () => {
254+
const { server, workerPid } = await spawnTree(false)
255+
expect(server.pid).toBeTruthy()
256+
expect(isRunning(server.pid!)).toBe(true)
257+
expect(isRunning(workerPid)).toBe(true)
258+
259+
killProcessTree(server.pid)
260+
await new Promise((r) => setTimeout(r, 2000))
261+
262+
expect(isRunning(server.pid!)).toBe(false)
263+
expect(isRunning(workerPid)).toBe(false)
264+
},
265+
15000
266+
)
267+
268+
test.skipIf(process.platform === 'win32')(
269+
'a bare single-process kill orphans an in-group worker (POSIX, why tree-kill is needed)',
270+
async () => {
271+
const { server, workerPid } = await spawnTree(false)
272+
expect(isRunning(workerPid)).toBe(true)
273+
274+
// signalling only the leader's PID leaves the rest of the group running
275+
process.kill(server.pid!, 'SIGKILL')
276+
await new Promise((r) => setTimeout(r, 2000))
277+
278+
expect(isRunning(server.pid!)).toBe(false)
279+
expect(isRunning(workerPid)).toBe(true)
280+
// (afterEach reaps the orphaned worker)
281+
},
282+
15000
283+
)
284+
})

tests/test/tests/process-tree.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { spawn } from 'node:child_process'
2+
3+
/**
4+
* Tree-kill a spawned dev server together with the worker processes it forked,
5+
* cross-platform. Shared by the spawn-based tests in this directory so they all
6+
* clean up the same (correct) way — mirrors `killProcessTree` in
7+
* packages/test/src/setup.ts.
8+
*
9+
* POSIX: signal the whole process group via the negative PID. This requires the
10+
* server to have been spawned with `detached: true` so it leads its own group;
11+
* the workers it forks inherit that group and are signalled too. If the pid is
12+
* not a group leader the negative-pid call throws and we fall back to signalling
13+
* the single process. Pure Node — no shell-out.
14+
*
15+
* Windows: there is no process-group kill (`process.kill(-pid)` throws with
16+
* ESRCH), and `process.kill(pid)` maps to TerminateProcess, which ends only the
17+
* root — a worker that broke away from libuv's job object would be orphaned.
18+
* `taskkill /F /T` walks the PID tree and ends those workers too. (Node has no
19+
* built-in tree-kill, so this one shell-out is unavoidable for a robust result.)
20+
*/
21+
export function killProcessTree(pid: number | undefined): void {
22+
if (!pid) return
23+
24+
if (process.platform === 'win32') {
25+
try {
26+
spawn('taskkill', ['/F', '/T', '/PID', String(pid)], { stdio: 'ignore' })
27+
} catch {
28+
// process already gone — fine
29+
}
30+
return
31+
}
32+
33+
// POSIX: graceful group SIGTERM, then a group SIGKILL fallback. The inner
34+
// fallbacks signal the single process when the pid is not a group leader.
35+
try {
36+
process.kill(-pid, 'SIGTERM')
37+
} catch {
38+
try {
39+
process.kill(pid, 'SIGTERM')
40+
} catch {
41+
// already gone
42+
}
43+
}
44+
setTimeout(() => {
45+
try {
46+
process.kill(-pid, 'SIGKILL')
47+
} catch {
48+
try {
49+
process.kill(pid, 'SIGKILL')
50+
} catch {
51+
// already gone
52+
}
53+
}
54+
}, 1000)
55+
}

0 commit comments

Comments
 (0)