Skip to content

Commit c9bcccb

Browse files
jaysoojaysoo
committed
Revert "fix(core): kill discrete tasks and use tree-kill for batch cleanup on SIGINT (#35175)"
This reverts commit d0f8012.
1 parent 3a264f4 commit c9bcccb

5 files changed

Lines changed: 21 additions & 390 deletions

File tree

‎e2e/nx/src/run.test.ts‎

Lines changed: 0 additions & 340 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {
22
checkFilesExist,
33
cleanupProject,
44
fileExists,
5-
getStrippedEnvironmentVariables,
65
isWindows,
76
newProject,
87
readJson,
@@ -15,7 +14,6 @@ import {
1514
updateFile,
1615
updateJson,
1716
} from '@nx/e2e-utils';
18-
import { execSync } from 'child_process';
1917
import { PackageJson } from 'nx/src/utils/package-json';
2018
import * as path from 'path';
2119

@@ -775,301 +773,6 @@ describe('Nx Running Tests', () => {
775773
});
776774
});
777775

778-
describe('SIGINT process cleanup', () => {
779-
if (isWindows()) {
780-
it('skipped on windows', () => {});
781-
} else {
782-
it('should kill executor-based (discrete) task processes on SIGINT', async () => {
783-
const lib = uniq('sigint-lib');
784-
runCLI(`generate @nx/js:lib libs/${lib}`);
785-
786-
const pidFile = path.join(tmpProjPath(), `libs/${lib}/build.pid`);
787-
const scriptBody = writePidScript('build.pid');
788-
789-
updateFile(`libs/${lib}/slow-build.js`, scriptBody);
790-
791-
// nx:run-script executor goes through the discrete/forked process path
792-
updateJson(`libs/${lib}/project.json`, (config) => {
793-
config.targets = {
794-
build: {
795-
executor: 'nx:run-script',
796-
options: { script: 'build' },
797-
},
798-
};
799-
return config;
800-
});
801-
802-
updateJson(`libs/${lib}/package.json`, (pkg) => {
803-
pkg.scripts = { build: 'node slow-build.js' };
804-
return pkg;
805-
});
806-
807-
const pids = await runNxAndSigint(
808-
['build', lib, '--skip-nx-cache'],
809-
pidFile
810-
);
811-
812-
for (const pid of pids) {
813-
expect(isProcessAlive(pid)).toBe(false);
814-
}
815-
}, 60_000);
816-
817-
it('should kill run-commands task processes on SIGINT', async () => {
818-
const lib = uniq('sigint-rc');
819-
runCLI(`generate @nx/js:lib libs/${lib}`);
820-
821-
const pidFile = path.join(tmpProjPath(), `libs/${lib}/build.pid`);
822-
823-
updateFile(`libs/${lib}/slow-build.js`, writePidScript('build.pid'));
824-
825-
updateJson(`libs/${lib}/project.json`, (config) => {
826-
config.targets = {
827-
build: {
828-
command: 'node slow-build.js',
829-
options: { cwd: `libs/${lib}` },
830-
},
831-
};
832-
return config;
833-
});
834-
835-
const pids = await runNxAndSigint(
836-
['build', lib, '--skip-nx-cache'],
837-
pidFile
838-
);
839-
840-
for (const pid of pids) {
841-
expect(isProcessAlive(pid)).toBe(false);
842-
}
843-
}, 60_000);
844-
845-
it('should kill continuous task processes on SIGINT', async () => {
846-
const lib = uniq('sigint-cont');
847-
runCLI(`generate @nx/js:lib libs/${lib}`);
848-
849-
const pidFile = path.join(tmpProjPath(), `libs/${lib}/serve.pid`);
850-
851-
updateFile(`libs/${lib}/serve.js`, writePidScript('serve.pid'));
852-
853-
updateJson(`libs/${lib}/project.json`, (config) => {
854-
config.targets = {
855-
serve: {
856-
command: 'node serve.js',
857-
options: { cwd: `libs/${lib}` },
858-
continuous: true,
859-
},
860-
};
861-
return config;
862-
});
863-
864-
const pids = await runNxAndSigint(
865-
['serve', lib, '--skip-nx-cache'],
866-
pidFile
867-
);
868-
869-
for (const pid of pids) {
870-
expect(isProcessAlive(pid)).toBe(false);
871-
}
872-
}, 60_000);
873-
874-
it('should kill executor-based (discrete) task processes on SIGINT (TUI mode)', async () => {
875-
const lib = uniq('sigint-tui');
876-
runCLI(`generate @nx/js:lib libs/${lib}`);
877-
878-
const pidFile = path.join(tmpProjPath(), `libs/${lib}/build.pid`);
879-
880-
updateFile(`libs/${lib}/slow-build.js`, writePidScript('build.pid'));
881-
882-
updateJson(`libs/${lib}/project.json`, (config) => {
883-
config.targets = {
884-
build: {
885-
executor: 'nx:run-script',
886-
options: { script: 'build' },
887-
},
888-
};
889-
return config;
890-
});
891-
892-
updateJson(`libs/${lib}/package.json`, (pkg) => {
893-
pkg.scripts = { build: 'node slow-build.js' };
894-
return pkg;
895-
});
896-
897-
const pids = await runNxAndSigint(
898-
['build', lib, '--skip-nx-cache'],
899-
pidFile,
900-
{ useTui: true }
901-
);
902-
903-
for (const pid of pids) {
904-
expect(isProcessAlive(pid)).toBe(false);
905-
}
906-
}, 60_000);
907-
908-
it('should kill run-commands task processes on SIGINT (TUI mode)', async () => {
909-
const lib = uniq('sigint-tui-rc');
910-
runCLI(`generate @nx/js:lib libs/${lib}`);
911-
912-
const pidFile = path.join(tmpProjPath(), `libs/${lib}/build.pid`);
913-
914-
updateFile(`libs/${lib}/slow-build.js`, writePidScript('build.pid'));
915-
916-
updateJson(`libs/${lib}/project.json`, (config) => {
917-
config.targets = {
918-
build: {
919-
command: 'node slow-build.js',
920-
options: { cwd: `libs/${lib}` },
921-
},
922-
};
923-
return config;
924-
});
925-
926-
const pids = await runNxAndSigint(
927-
['build', lib, '--skip-nx-cache'],
928-
pidFile,
929-
{ useTui: true }
930-
);
931-
932-
for (const pid of pids) {
933-
expect(isProcessAlive(pid)).toBe(false);
934-
}
935-
}, 60_000);
936-
937-
it('should kill continuous task processes on SIGINT (TUI mode)', async () => {
938-
const lib = uniq('sigint-tui-cont');
939-
runCLI(`generate @nx/js:lib libs/${lib}`);
940-
941-
const pidFile = path.join(tmpProjPath(), `libs/${lib}/serve.pid`);
942-
943-
updateFile(`libs/${lib}/serve.js`, writePidScript('serve.pid'));
944-
945-
updateJson(`libs/${lib}/project.json`, (config) => {
946-
config.targets = {
947-
serve: {
948-
command: 'node serve.js',
949-
options: { cwd: `libs/${lib}` },
950-
continuous: true,
951-
},
952-
};
953-
return config;
954-
});
955-
956-
const pids = await runNxAndSigint(
957-
['serve', lib, '--skip-nx-cache'],
958-
pidFile,
959-
{ useTui: true }
960-
);
961-
962-
for (const pid of pids) {
963-
expect(isProcessAlive(pid)).toBe(false);
964-
}
965-
}, 60_000);
966-
}
967-
968-
/** Script that writes its PID to a file and keeps running. */
969-
function writePidScript(filename: string): string {
970-
return `
971-
const fs = require('fs');
972-
const path = require('path');
973-
fs.writeFileSync(path.join(__dirname, '${filename}'), String(process.pid));
974-
setInterval(() => {}, 1000);
975-
`;
976-
}
977-
978-
/**
979-
* Runs nx inside a real pseudo-terminal, polls for a PID file,
980-
* sends SIGINT (identical to Ctrl+C), and asserts nx exits
981-
* promptly with all processes in the tree dead.
982-
*/
983-
async function runNxAndSigint(
984-
args: string[],
985-
pidFile: string,
986-
{ useTui }: { useTui?: boolean } = {}
987-
): Promise<number[]> {
988-
const { existsSync, readFileSync, unlinkSync } = require('fs');
989-
const { RustPseudoTerminal } = require('nx/src/native');
990-
991-
try {
992-
unlinkSync(pidFile);
993-
} catch {}
994-
995-
const nxBin = path.join(tmpProjPath(), 'node_modules', '.bin', 'nx');
996-
const command = `${nxBin} ${args.join(' ')}`;
997-
998-
const pty = new RustPseudoTerminal();
999-
const childProcess = pty.runCommand(command, tmpProjPath(), {
1000-
...getStrippedEnvironmentVariables(),
1001-
CI: 'true',
1002-
FORCE_COLOR: 'false',
1003-
NX_DAEMON: 'false',
1004-
NX_TUI: useTui ? 'true' : 'false',
1005-
...(useTui ? { NX_TUI_SKIP_CAPABILITY_CHECK: 'true' } : {}),
1006-
});
1007-
1008-
const nxPid = childProcess.getPid();
1009-
console.log(`[sigint-test] started PID=${nxPid}, useTui=${!!useTui}`);
1010-
1011-
// Track exit immediately so we don't miss it
1012-
let exited = false;
1013-
let exitResolve: (clean: boolean) => void;
1014-
const exitPromise = new Promise<boolean>((r) => (exitResolve = r));
1015-
childProcess.onExit((msg) => {
1016-
console.log(`[sigint-test] onExit: ${msg}`);
1017-
exited = true;
1018-
exitResolve(true);
1019-
});
1020-
1021-
// Poll for PID file written by the task script
1022-
let taskPid: number | undefined;
1023-
const startTime = Date.now();
1024-
while (Date.now() - startTime < 30_000 && !exited) {
1025-
if (existsSync(pidFile)) {
1026-
taskPid = parseInt(readFileSync(pidFile, 'utf-8').trim(), 10);
1027-
if (taskPid && !isNaN(taskPid)) break;
1028-
}
1029-
await new Promise((r) => setTimeout(r, 200));
1030-
}
1031-
1032-
if (!taskPid) {
1033-
childProcess.kill('SIGKILL');
1034-
throw new Error(
1035-
exited
1036-
? 'nx exited before task script wrote PID file'
1037-
: 'Timed out waiting for PID file from task'
1038-
);
1039-
}
1040-
1041-
expect(isProcessAlive(taskPid)).toBe(true);
1042-
1043-
// Capture entire process tree before sending SIGINT
1044-
const processTree = getProcessTree(nxPid);
1045-
1046-
console.log(
1047-
`[sigint-test] taskPid=${taskPid} alive=${isProcessAlive(taskPid)}, tree=${JSON.stringify(processTree)}`
1048-
);
1049-
1050-
// Send SIGINT — identical to Ctrl+C in a real terminal
1051-
console.log(`[sigint-test] sending SIGINT`);
1052-
childProcess.kill('SIGINT');
1053-
1054-
// Nx should exit promptly. Without the fix, cleanup hangs
1055-
// waiting for unkilled discrete tasks.
1056-
const exitTimeout = setTimeout(() => {
1057-
console.log(`[sigint-test] TIMEOUT — sending SIGKILL`);
1058-
childProcess.kill('SIGKILL');
1059-
exitResolve(false);
1060-
}, 10_000);
1061-
const exitedCleanly = await exitPromise;
1062-
clearTimeout(exitTimeout);
1063-
1064-
console.log(`[sigint-test] exitedCleanly=${exitedCleanly}`);
1065-
expect(exitedCleanly).toBe(true);
1066-
1067-
await new Promise((r) => setTimeout(r, 2000));
1068-
1069-
return processTree;
1070-
}
1071-
});
1072-
1073776
describe('exec', () => {
1074777
let pkg: string;
1075778
let pkg2: string;
@@ -1242,46 +945,3 @@ describe('Nx Running Tests', () => {
1242945
});
1243946
});
1244947
});
1245-
1246-
function isProcessAlive(pid: number): boolean {
1247-
try {
1248-
process.kill(pid, 0);
1249-
return true;
1250-
} catch {
1251-
return false;
1252-
}
1253-
}
1254-
1255-
/**
1256-
* Get all descendant PIDs of a process (recursive).
1257-
* Uses `pgrep -P` on macOS/Linux.
1258-
*/
1259-
function getProcessTree(rootPid: number): number[] {
1260-
const pids: number[] = [rootPid];
1261-
try {
1262-
const children = execSync(`pgrep -P ${rootPid}`, { encoding: 'utf-8' })
1263-
.trim()
1264-
.split('\n')
1265-
.filter(Boolean)
1266-
.map(Number);
1267-
for (const child of children) {
1268-
pids.push(...getProcessTree(child));
1269-
}
1270-
} catch {
1271-
// No children or process already exited
1272-
}
1273-
return pids;
1274-
}
1275-
1276-
/**
1277-
* Kill an entire process tree by walking descendants.
1278-
* Used as a fallback when cleanup times out.
1279-
*/
1280-
function killProcessTree(pid: number) {
1281-
const pids = getProcessTree(pid);
1282-
for (const p of pids) {
1283-
try {
1284-
process.kill(p, 'SIGKILL');
1285-
} catch {}
1286-
}
1287-
}

0 commit comments

Comments
 (0)