diff --git a/.changeset/bounded-version-probe.md b/.changeset/bounded-version-probe.md new file mode 100644 index 00000000..17d971e3 --- /dev/null +++ b/.changeset/bounded-version-probe.md @@ -0,0 +1,5 @@ +--- +'@tanstack/create': patch +--- + +Bound optional package-manager version detection and clean up owned subprocesses on timeout, cancellation, or caller exit, while retaining the unknown-version fallback. diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index ca506976..6d9cb29f 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -29,6 +29,20 @@ jobs: run: pnpm build - name: Test Unit run: pnpm test:unit + test-version-probe-windows: + name: Test (Windows Version Probe) + runs-on: windows-latest + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + - name: Setup Tools + uses: tanstack/config/.github/setup@e4b48f16568324f76f467aa4c2aac2f05db632c3 # main + - name: Build core + run: pnpm --filter @tanstack/create build + - name: Test actual package-manager shims + run: pnpm --filter @tanstack/create exec vitest run tests/environment-version-probe.test.ts provenance: name: Provenance runs-on: ubuntu-latest diff --git a/packages/create/package.json b/packages/create/package.json index 699584e6..577678b3 100644 --- a/packages/create/package.json +++ b/packages/create/package.json @@ -88,6 +88,7 @@ "@vitest/coverage-v8": "4.1.5", "eslint": "^9.20.0", "memfs": "4.17.0", + "tsx": "^4.19.4", "typescript": "^6.0.2", "vitest": "^4.1.5", "vitest-fetch-mock": "^0.4.5" diff --git a/packages/create/src/environment.ts b/packages/create/src/environment.ts index fc5bf4f5..9c58bd19 100644 --- a/packages/create/src/environment.ts +++ b/packages/create/src/environment.ts @@ -19,7 +19,30 @@ import type { Environment } from './types.js' export type { MemoryEnvironmentOutput } from './edge-environment.js' -export function createDefaultEnvironment(cancelSignal?: AbortSignal): Environment { +// The live supervisor owns its process group. Only it signals that group, so +// cleanup never targets a numeric group ID after its owner has exited. +const versionProbeSupervisor = ` +const { spawn } = require('node:child_process') +const stop = () => process.kill(-process.pid, 'SIGKILL') +process.on('disconnect', stop) +process.on('SIGTERM', stop) +process.on('SIGCONT', stop) +process.on('SIGINT', stop) +if (!process.connected) stop() +const child = spawn(process.argv[1], ['--version'], { stdio: ['ignore', 'pipe', 'pipe'] }) +child.stdout.pipe(process.stdout) +child.stderr.pipe(process.stderr) +child.on('error', () => { process.exitCode = 1 }) +child.on('close', (code) => { + process.exitCode = code ?? 1 + process.removeListener('disconnect', stop) + if (process.connected) process.disconnect() +}) +` + +export function createDefaultEnvironment( + cancelSignal?: AbortSignal, +): Environment { let errors: Array = [] return { startRun: () => { @@ -45,11 +68,86 @@ export function createDefaultEnvironment(cancelSignal?: AbortSignal): Environmen return writeFile(path, getBinaryFile(base64Contents) as string) }, getPackageManagerVersion: async (packageManager, cwd) => { + if (cancelSignal?.aborted) return undefined + // Cancel through stop() while Windows can still inspect the live tree. + const posix = process.platform !== 'win32' + const subprocess = posix + ? execa( + process.execPath, + [ + '--input-type=commonjs', + '-e', + versionProbeSupervisor, + packageManager, + ], + { + cwd, + detached: true, + ipc: true, + killSignal: 'SIGCONT', + forceKillAfterDelay: 1_000, + }, + ) + : execa(packageManager, ['--version'], { cwd }) + let cleanup: Promise | undefined + const stop = () => { + cleanup ??= (async () => { + if (subprocess.pid !== undefined) { + try { + if (process.platform === 'win32') { + if ( + subprocess.exitCode === null && + subprocess.signalCode === null + ) { + await execa( + 'taskkill', + ['/PID', String(subprocess.pid), '/T', '/F'], + { + timeout: 5_000, + windowsHide: true, + }, + ) + } + } else { + // Resume a stopped owner so it can kill its own group. Execa + // escalates to SIGKILL after one second if it cannot respond. + subprocess.kill() + if (subprocess.connected) subprocess.disconnect() + } + } catch { + subprocess.kill('SIGKILL') + } + } + subprocess.stdout.destroy() + subprocess.stderr.destroy() + await subprocess.catch(() => undefined) + return undefined + })() + return cleanup + } + let timer: ReturnType | undefined + const timeout = new Promise((resolveTimeout) => { + timer = setTimeout(() => { + void stop().then(() => resolveTimeout(undefined)) + }, 15_000) + }) + let onAbort: (() => void) | undefined + const cancellation = new Promise((resolveCancellation) => { + onAbort = () => { + void stop().then(() => resolveCancellation(undefined)) + } + cancelSignal?.addEventListener('abort', onAbort, { once: true }) + if (cancelSignal?.aborted) onAbort() + }) try { - const result = await execa(packageManager, ['--version'], { cwd, cancelSignal }) - return result.stdout.trim() + const result = await Promise.race([subprocess, timeout, cancellation]) + return result?.stdout.trim() } catch { return undefined + } finally { + clearTimeout(timer) + if (onAbort) cancelSignal?.removeEventListener('abort', onAbort) + if (cleanup) await cleanup } }, execute: async ( @@ -117,8 +215,9 @@ export function createDefaultEnvironment(cancelSignal?: AbortSignal): Environmen } export function createMemoryEnvironment(returnPathsRelativeTo: string = '') { - const { environment, output } = - createEdgeMemoryEnvironment(returnPathsRelativeTo) + const { environment, output } = createEdgeMemoryEnvironment( + returnPathsRelativeTo, + ) const resolvePath = (path: string) => resolve(process.cwd(), path) const appendFile = environment.appendFile diff --git a/packages/create/tests/environment-version-probe.test.ts b/packages/create/tests/environment-version-probe.test.ts new file mode 100644 index 00000000..e90d020b --- /dev/null +++ b/packages/create/tests/environment-version-probe.test.ts @@ -0,0 +1,402 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + chmod, + mkdtemp, + readFile, + realpath, + rm, + writeFile, +} from 'node:fs/promises' +import { delimiter, dirname, join } from 'node:path' +import { tmpdir } from 'node:os' +import { spawn } from 'node:child_process' +import { existsSync } from 'node:fs' + +import { createDefaultEnvironment } from '../src/environment.js' + +let directory: string | undefined +let pidFile: string | undefined +let probePending = false + +async function installShim(body: string) { + directory = await mkdtemp(join(tmpdir(), 'tanstack-version-probe-')) + pidFile = join(directory, 'probe.pid') + const script = join(directory, 'pnpm') + const contents = [ + '#!/usr/bin/env node', + "import { writeFileSync } from 'node:fs'", + 'writeFileSync(' + JSON.stringify(pidFile) + ', String(process.pid))', + 'writeFileSync(' + + JSON.stringify(join(directory, 'cwd')) + + ', process.cwd())', + body, + ].join('\n') + await writeFile(join(directory, 'package.json'), '{"type":"module"}') + await writeFile(script, contents) + await chmod(script, 0o755) + if (process.platform === 'win32') { + await writeFile( + join(directory, 'pnpm.cmd'), + '@"' + process.execPath + '" "' + script + '" %*\r\n', + ) + } + vi.stubEnv( + 'PATH', + [directory, dirname(process.execPath), process.env.PATH ?? ''].join( + delimiter, + ), + ) + return directory +} + +afterEach(async () => { + vi.unstubAllEnvs() + if (pidFile && probePending) { + try { + const pid = Number(await readFile(pidFile, 'utf8')) + // Clean up only the child created by this fixture if an assertion failed. + try { + process.kill(pid, 'SIGKILL') + } catch {} + } catch {} + } + if (directory) { + try { + const pid = Number( + await readFile(join(directory, 'descendant.pid'), 'utf8'), + ) + try { + process.kill(pid, 'SIGKILL') + } catch {} + } catch {} + await rm(directory, { recursive: true, force: true }) + } + directory = undefined + pidFile = undefined +}) + +async function probe( + environment: ReturnType, + cwd: string, +) { + probePending = true + try { + return await environment.getPackageManagerVersion?.('pnpm', cwd) + } finally { + probePending = false + } +} + +describe('package-manager version subprocess', () => { + it('terminates descendants holding the probe pipes open', async () => { + const cwd = await installShim( + "const { spawn } = await import('node:child_process');" + + "const child = spawn(process.execPath, ['-e', " + + JSON.stringify( + "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)", + ) + + "], { stdio: ['ignore', 'inherit', 'inherit'] });" + + "writeFileSync('descendant.pid', String(child.pid)); setInterval(() => {}, 1000)", + ) + const environment = createDefaultEnvironment() + const started = Date.now() + await expect(probe(environment, cwd)).resolves.toBeUndefined() + expect(Date.now() - started).toBeLessThan(23_000) + const pid = Number(await readFile(join(cwd, 'descendant.pid'), 'utf8')) + await expect + .poll( + () => { + try { + process.kill(pid, 0) + return false + } catch { + return true + } + }, + { timeout: 2_000 }, + ) + .toBe(true) + // Remove the receipt so cleanup cannot target a later reused PID. + await rm(join(cwd, 'descendant.pid')) + expect(environment.getErrors()).toEqual([]) + }, 30_000) + + it('cancels the owned probe tree before returning the fallback', async () => { + const cwd = await installShim( + "const { spawn } = await import('node:child_process');" + + "const child = spawn(process.execPath, ['-e', " + + JSON.stringify( + "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)", + ) + + "], { stdio: ['ignore', 'inherit', 'inherit'] });" + + "writeFileSync('descendant.pid', String(child.pid)); setInterval(() => {}, 1000)", + ) + const controller = new AbortController() + const environment = createDefaultEnvironment(controller.signal) + const pending = probe(environment, cwd) + await expect + .poll( + async () => { + try { + await readFile(join(cwd, 'descendant.pid'), 'utf8') + return true + } catch { + return false + } + }, + { timeout: 5_000 }, + ) + .toBe(true) + const descendant = Number( + await readFile(join(cwd, 'descendant.pid'), 'utf8'), + ) + const started = Date.now() + controller.abort() + await expect(pending).resolves.toBeUndefined() + expect(Date.now() - started).toBeLessThan(7_000) + await expect + .poll( + () => { + try { + process.kill(descendant, 0) + return false + } catch { + return true + } + }, + { timeout: 2_000 }, + ) + .toBe(true) + await rm(join(cwd, 'descendant.pid')) + expect(environment.getErrors()).toEqual([]) + }, 25_000) + + it.skipIf(process.platform === 'win32')( + 'cleans up the group when the calling process exits', + async () => { + const cwd = await installShim( + "const { spawn } = await import('node:child_process');" + + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: ['ignore', 'inherit', 'inherit'] });" + + "writeFileSync('descendant.pid', String(child.pid)); setInterval(() => {}, 1000)", + ) + const entry = new URL('../src/environment.ts', import.meta.url).href + const receipt = join(cwd, 'descendant.pid') + probePending = true + const caller = spawn( + process.execPath, + [ + '--import', + import.meta.resolve('tsx'), + '--input-type=module', + '-e', + ` + import { existsSync } from 'node:fs' + const { createDefaultEnvironment } = await import(${JSON.stringify(entry)}) + void createDefaultEnvironment().getPackageManagerVersion('pnpm', ${JSON.stringify(cwd)}) + setInterval(() => { if (existsSync(${JSON.stringify(receipt)})) process.exit(0) }, 20) + `, + ], + { cwd, stdio: 'ignore' }, + ) + try { + await new Promise((resolveExit, reject) => { + const timer = setTimeout(() => { + caller.kill('SIGKILL') + reject(new Error('Caller did not finish its fixture')) + }, 5_000) + caller.once('error', (error) => { + clearTimeout(timer) + reject(error) + }) + caller.once('exit', (code) => { + clearTimeout(timer) + if (code === 0) resolveExit() + else reject(new Error(`Caller exit ${code}`)) + }) + }) + const pids = [ + Number(await readFile(join(cwd, 'probe.pid'), 'utf8')), + Number(await readFile(receipt, 'utf8')), + ] + for (const pid of pids) { + await expect + .poll( + () => { + try { + process.kill(pid, 0) + return false + } catch { + return true + } + }, + { timeout: 2_000 }, + ) + .toBe(true) + } + await rm(receipt) + probePending = false + } finally { + caller.kill('SIGKILL') + } + }, + 15_000, + ) + + it.skipIf(process.platform === 'win32')( + 'stops descendants after the shim has exited without signalling an old group ID', + async () => { + const cwd = await installShim( + "const { spawn } = await import('node:child_process');" + + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: ['ignore', 'inherit', 'inherit'] });" + + "writeFileSync('descendant.pid', String(child.pid)); process.exit(0)", + ) + const controller = new AbortController() + const pending = probe(createDefaultEnvironment(controller.signal), cwd) + await expect + .poll( + async () => { + try { + const pid = Number(await readFile(join(cwd, 'probe.pid'), 'utf8')) + process.kill(pid, 0) + return false + } catch { + return existsSync(join(cwd, 'descendant.pid')) + } + }, + { timeout: 5_000 }, + ) + .toBe(true) + const kill = vi.spyOn(process, 'kill') + try { + controller.abort() + await expect(pending).resolves.toBeUndefined() + expect(kill.mock.calls.some(([pid]) => pid < 0)).toBe(false) + } finally { + kill.mockRestore() + } + const descendant = Number( + await readFile(join(cwd, 'descendant.pid'), 'utf8'), + ) + await expect + .poll( + () => { + try { + process.kill(descendant, 0) + return false + } catch { + return true + } + }, + { timeout: 2_000 }, + ) + .toBe(true) + await rm(join(cwd, 'descendant.pid')) + }, + 15_000, + ) + + it.skipIf(process.platform === 'win32')( + 'resumes a stopped supervisor so cancellation can clean up its group', + async () => { + const cwd = await installShim( + "const { spawn } = await import('node:child_process');" + + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: ['ignore', 'inherit', 'inherit'] });" + + "writeFileSync('descendant.pid', String(child.pid));" + + "process.kill(process.ppid, 'SIGSTOP');" + + "writeFileSync('supervisor.pid', String(process.ppid)); setInterval(() => {}, 1000)", + ) + const controller = new AbortController() + const pending = probe(createDefaultEnvironment(controller.signal), cwd) + await expect + .poll(() => existsSync(join(cwd, 'supervisor.pid')), { timeout: 5_000 }) + .toBe(true) + const supervisor = Number( + await readFile(join(cwd, 'supervisor.pid'), 'utf8'), + ) + let timer: ReturnType | undefined + try { + controller.abort() + await expect( + Promise.race([ + pending, + new Promise((_, reject) => { + timer = setTimeout( + () => + reject(new Error('Stopped supervisor blocked cancellation')), + 2_000, + ) + }), + ]), + ).resolves.toBeUndefined() + } finally { + clearTimeout(timer) + // Recover only this fixture if the regression leaves its owner stopped. + try { + process.kill(supervisor, 'SIGCONT') + } catch {} + await pending + } + const descendant = Number( + await readFile(join(cwd, 'descendant.pid'), 'utf8'), + ) + await expect + .poll( + () => { + try { + process.kill(descendant, 0) + return false + } catch { + return true + } + }, + { timeout: 2_000 }, + ) + .toBe(true) + await rm(join(cwd, 'descendant.pid')) + }, + 15_000, + ) + + it('does not launch a probe after cancellation', async () => { + const cwd = await installShim('setInterval(() => {}, 1000)') + const controller = new AbortController() + controller.abort() + await expect( + probe(createDefaultEnvironment(controller.signal), cwd), + ).resolves.toBeUndefined() + await expect(readFile(join(cwd, 'probe.pid'), 'utf8')).rejects.toThrow() + }) + + it('reads and trims the version in the target project', async () => { + const cwd = await installShim( + "process.stdout.write(' 10.2.0 ' + String.fromCharCode(10))", + ) + const environment = createDefaultEnvironment() + await expect(probe(environment, cwd)).resolves.toBe('10.2.0') + expect(await realpath(await readFile(join(cwd, 'cwd'), 'utf8'))).toBe( + await realpath(cwd), + ) + expect(environment.getErrors()).toEqual([]) + }) + + it('stops a stalled optional probe and keeps the unknown-version fallback', async () => { + const cwd = await installShim('setInterval(() => {}, 1000)') + const environment = createDefaultEnvironment() + await expect(probe(environment, cwd)).resolves.toBeUndefined() + const pid = Number(await readFile(join(cwd, 'probe.pid'), 'utf8')) + await expect + .poll( + () => { + try { + process.kill(pid, 0) + return false + } catch { + return true + } + }, + { timeout: 2_000 }, + ) + .toBe(true) + expect(environment.getErrors()).toEqual([]) + }, 30_000) +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 766689b5..b40484f0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -291,6 +291,9 @@ importers: memfs: specifier: 4.17.0 version: 4.17.0 + tsx: + specifier: ^4.19.4 + version: 4.19.4 typescript: specifier: ^6.0.2 version: 6.0.2