Skip to content

Commit 962b23e

Browse files
committed
Simplify timeout logic
This commit simplifies the timeout logic by no longer passing `timeout` to `spawn`. Instead, the full timeout logic is handled by `run`, which sets a timeout and determines whether the process was killed. Now that there is no duplicate logic, `wasKilledByTimeout` can be removed in favor of checking `killedByTimeout` directly.
1 parent 24fc64e commit 962b23e

2 files changed

Lines changed: 9 additions & 25 deletions

File tree

lib/java-caller.js

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -107,25 +107,6 @@ class JavaCaller {
107107
let timeoutId;
108108
let killedByTimeout = false;
109109

110-
const wasKilledByTimeout = (code, signal) => {
111-
if (!runOptions.timeout) {
112-
return false;
113-
}
114-
if (killedByTimeout) {
115-
return true;
116-
}
117-
if (signal && signal === runOptions.killSignal) {
118-
return true;
119-
}
120-
const signals = os.constants && os.constants.signals ? os.constants.signals : {};
121-
if (typeof runOptions.killSignal === "string" && signals[runOptions.killSignal] && code === 128 + signals[runOptions.killSignal]) {
122-
return true;
123-
}
124-
if (typeof runOptions.killSignal === "number" && code === 128 + runOptions.killSignal) {
125-
return true;
126-
}
127-
return false;
128-
};
129110
const prom = new Promise((resolve) => {
130111
// Spawn java command line
131112
debug(`Java command: ${javaExeToUse} ${javaArgs.join(" ")}`);
@@ -136,8 +117,6 @@ class JavaCaller {
136117
stdio: this.output === "console" ? "inherit" : runOptions.detached ? "ignore" : "pipe",
137118
windowsHide: true,
138119
windowsVerbatimArguments: runOptions.windowsVerbatimArguments,
139-
timeout: runOptions.timeout,
140-
killSignal: runOptions.killSignal,
141120
};
142121
if (javaExeToUse.includes(" ")) {
143122
spawnOptions.shell = true;
@@ -147,9 +126,9 @@ class JavaCaller {
147126
if (runOptions.timeout) {
148127
timeoutId = setTimeout(() => {
149128
if (!child.killed) {
150-
killedByTimeout = true;
151129
try {
152130
child.kill(runOptions.killSignal);
131+
killedByTimeout = true;
153132
} catch (err) {
154133
stderr += `Failed to kill process after ${runOptions.timeout}ms: ${err.message}`;
155134
}
@@ -172,6 +151,11 @@ class JavaCaller {
172151
child.on("error", (data) => {
173152
this.status = 666;
174153
stderr += "Java spawn error: " + data;
154+
155+
if (timeoutId) {
156+
clearTimeout(timeoutId);
157+
}
158+
175159
resolve();
176160
});
177161

@@ -181,8 +165,8 @@ class JavaCaller {
181165
clearTimeout(timeoutId);
182166
}
183167

184-
if (wasKilledByTimeout(code, signal)) {
185-
// Process was terminated because of the timeout, either via our fallback timer or the built-in spawn timeout
168+
if (killedByTimeout) {
169+
// Process was terminated because of the timeout
186170
this.status = 666;
187171
stderr += `Process timed out with ${runOptions.killSignal} after ${runOptions.timeout}ms.`;
188172
} else {

test/java-caller.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ describe("Call with classes", () => {
200200
checkStdOutIncludes(`JavaCallerTester is called !`, stdout, stderr);
201201
});
202202

203-
it("should terminate once timeout is reached", async () => {
203+
it("should terminate once timeout is reached", async () => {
204204
const java = new JavaCaller({
205205
classPath: 'test/java/dist',
206206
mainClass: 'com.nvuillam.javacaller.JavaCallerTester'

0 commit comments

Comments
 (0)