Skip to content

Commit b00c6b1

Browse files
committed
Prevent CI hangs from unbounded subprocess waits (#1434)
Two intermittent CI hang issues were identified: 1. test-message-generation-bin.js: The before() hook runs spawnSync for npm init, npm pack, and npm install (full native rebuild from tarball) with no timeout. If any of these hangs (network issue, lock contention, slow arm64 runner), mocha blocks indefinitely until the GitHub Actions runner is reclaimed, producing a "context canceled" error with no useful diagnostics. Fixed by adding timeout to all spawnSync calls (60s for init, 120s for pack/scripts, 300s for install). 2. test/electron/run_test.js: The parent process spawns Electron via xvfb-run and waits for the close event. If the Electron child process or lingering DDS/FastDDS background threads don't terminate cleanly (due to async app.quit() racing with process.exit(), or xvfb-run holding stdio pipes open), the parent hangs forever. Fixed by adding a 30-second kill timeout that forcibly terminates the child if it doesn't exit after the test completes. Fix: #1433
1 parent 10706b9 commit b00c6b1

2 files changed

Lines changed: 66 additions & 6 deletions

File tree

test/electron/run_test.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,32 @@ console.log('Launching Electron to run test_usability.js...');
2828
const child = spawn(command, args, {
2929
stdio: 'inherit',
3030
env: { ...process.env, ELECTRON_ENABLE_LOGGING: true },
31+
detached: true,
3132
});
3233

33-
child.on('close', (code) => {
34-
console.log(`Electron process exited with code ${code}`);
34+
// Kill the child process tree if it doesn't exit within 30 seconds
35+
const killTimeout = setTimeout(() => {
36+
console.error('Electron process did not exit in time, killing...');
37+
try {
38+
process.kill(-child.pid, 'SIGKILL');
39+
} catch (err) {
40+
try {
41+
child.kill('SIGKILL');
42+
} catch (e) {
43+
// Process already dead
44+
}
45+
}
46+
}, 30 * 1000);
47+
48+
child.on('close', (code, signal) => {
49+
clearTimeout(killTimeout);
50+
if (code !== null) {
51+
console.log(`Electron process exited with code ${code}`);
52+
} else {
53+
console.log(
54+
`Electron process was terminated by signal ${signal || 'unknown'}`
55+
);
56+
}
3557
if (code === 0) {
3658
console.log('Test Passed!');
3759
process.exit(0);

test/test-message-generation-bin.js

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,39 @@ function getNodeVersionInfo() {
3333
.map((x) => parseInt(x));
3434
}
3535

36+
function runCommand(command, args, options) {
37+
const result = childProcess.spawnSync(command, args, options);
38+
if (
39+
result.error ||
40+
(typeof result.status === 'number' && result.status !== 0)
41+
) {
42+
const parts = [
43+
`Failed to run: ${command} ${Array.isArray(args) ? args.join(' ') : ''}`,
44+
];
45+
if (
46+
result.error &&
47+
result.error.code === 'ETIMEDOUT' &&
48+
options &&
49+
options.timeout
50+
) {
51+
parts.push(`Command timed out after ${options.timeout} ms`);
52+
} else if (result.error) {
53+
parts.push(`Error: ${result.error.message}`);
54+
}
55+
if (typeof result.status === 'number' && result.status !== 0) {
56+
parts.push(`Exit status: ${result.status}`);
57+
}
58+
if (result.stdout) {
59+
parts.push(`stdout:\n${result.stdout.toString()}`);
60+
}
61+
if (result.stderr) {
62+
parts.push(`stderr:\n${result.stderr.toString()}`);
63+
}
64+
throw new Error(parts.join('\n'));
65+
}
66+
return result;
67+
}
68+
3669
describe('rclnodejs generate-messages binary-script tests', function () {
3770
let cwd;
3871
let tmpPkg;
@@ -60,11 +93,13 @@ describe('rclnodejs generate-messages binary-script tests', function () {
6093
// stdio: 'inherit',
6194
shell: true,
6295
cwd: this.tmpPkg,
96+
timeout: 60 * 1000,
6397
});
64-
childProcess.spawnSync('npm', ['pack', this.cwd], {
98+
runCommand('npm', ['pack', this.cwd], {
6599
// stdio: 'inherit',
66100
shell: true,
67101
cwd: this.tmpPkg,
102+
timeout: 120 * 1000,
68103
});
69104

70105
let tgz;
@@ -80,10 +115,11 @@ describe('rclnodejs generate-messages binary-script tests', function () {
80115
return;
81116
}
82117
let tgzPath = path.join(this.tmpPkg, tgz);
83-
childProcess.spawnSync('npm', ['install', tgzPath], {
118+
runCommand('npm', ['install', tgzPath], {
84119
// stdio: 'inherit',
85120
shell: true,
86121
cwd: this.tmpPkg,
122+
timeout: 300 * 1000,
87123
});
88124

89125
let generatedFolderPath = createGeneratedFolderPath(this.tmpPkg);
@@ -114,9 +150,10 @@ describe('rclnodejs generate-messages binary-script tests', function () {
114150

115151
it('test generate-ros-messages script operation', function (done) {
116152
let script = createScriptFolderPath(this.tmpPkg);
117-
childProcess.spawnSync(script, args, {
153+
runCommand(script, args, {
118154
// stdio: 'inherit',
119155
shell: true,
156+
timeout: 120 * 1000,
120157
});
121158

122159
let generatedFolderPath = createGeneratedFolderPath(this.tmpPkg);
@@ -132,10 +169,11 @@ describe('rclnodejs generate-messages binary-script tests', function () {
132169
});
133170

134171
it('test npx generate-ros-messages script operation', function (done) {
135-
childProcess.spawnSync('npx', [SCRIPT_NAME, ...args], {
172+
runCommand('npx', [SCRIPT_NAME, ...args], {
136173
// stdio: 'inherit',
137174
shell: true,
138175
cwd: this.tmpPkg,
176+
timeout: 120 * 1000,
139177
});
140178

141179
let generatedFolderPath = createGeneratedFolderPath(this.tmpPkg);

0 commit comments

Comments
 (0)