Skip to content

Commit bbe6af2

Browse files
BrendanC23nvuillam
andauthored
Add timeout (#147)
* Add runOptions.timeout This commit adds new `runOptions.timeout` and `runOptions.killSignal` options that allow a process to be killed after a specified number of milliseconds have elapsed. See the [Node ChildProcess docs](https://nodejs.org/api/child_process.html#child_processspawncommand-args-options). This commit includes several additional changes that were necessary as part of the implementation of `timeout`: * Fix to `JavaCallerTester.java` that replaces an `==` string comparison with a `args[0].equals("--sleep")`. This fixes the `sleep` command line argument, which previously never triggered because the comparison was always false. * Recompile `JavaCallerTester.class` and `JavaCallerTester.jar` * Fixed `should call JavaCallerTester.class detached` test. This began failing after the call fix for `--sleep`. The test has been rewritten to demonstrate the expected behavior for `detached`. Now, the test will check that the Java process is initially still running and then check the return status code once it exits. * Update the NPM script `java:compile` to use `--release 8` instead of `-source 8 -target 1.8`. This fixes an error that occurred when running the previous script: >warning: [options] bootstrap class path is not set in conjunction with -source 8 not setting the bootstrap class path may lead to class files that cannot run on JDK 8 --release 8 is recommended instead of -source 8 -target 1.8 because it sets the bootstrap class path automatically See [this StackOverflow post](https://stackoverflow.com/a/61715683) for more details. Note that a warning is still generated: >warning: [options] target value 8 is obsolete and will be removed in a future release warning: [options] To suppress warnings about obsolete options, use -Xlint:-options. Fixes #144 * Add to changelog * Add timeout options to JavaCallerRunOptions type * Remove null check for code when checking for timeout This commit removes the check for `exitCode === null`, which will only be the case on Windows. On Linux, the `exitCode` will be populated. The [Node docs](https://nodejs.org/api/child_process.html#event-exit), say: >The exit code if the child process exited on its own, or null if the child process terminated due to a signal. >The 'exit' event is emitted after the child process ends. If the process exited, code is the final exit code of the process, otherwise null. * Fix killSignal default in TypeScript docs * Remove no-undef ESLint check for *.ts files As recommended in the [TypeScript ESLint docs](https://typescript-eslint.io/troubleshooting/faqs/eslint#i-get-errors-from-the-no-undef-rule-about-global-variables-not-being-defined-even-though-there-are-no-typescript-errors), the `no-undef` rule should not be used for `*.ts` files because it gives false positives. This causes issues with using `NodeJS.Signals` in `index.d.ts`. * Add temporary logging * Handle cross-platform timeout * unify timeout codes * 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. * Remove unused variable --------- Co-authored-by: Nicolas Vuillamy <nicolas.vuillamy@gmail.com>
1 parent 639f49b commit bbe6af2

File tree

11 files changed

+112
-12
lines changed

11 files changed

+112
-12
lines changed

.eslintrc.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ module.exports = {
3434
files: ["**/*.d.ts"],
3535
parser: "@typescript-eslint/parser",
3636
rules: {
37-
"getter-return": "off"
37+
"getter-return": "off",
38+
"no-undef": "off"
3839
}
3940
}
4041
],

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## Unreleased
44

5+
- Add `timeout` and `killSignal` run options
6+
57
## [4.3.3] 2025-02-03
68

79
- Add types definition to make library compliant with typescript usage

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,15 @@ Example: `["-Xms256m", "--someflagwithvalue myVal", "-c"]`
6565

6666
| Parameter | Description | Default | Example |
6767
|-----------|-------------|---------|---------|
68-
| [detached](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) | If set to true, node will node wait for the java command to be completed.<br/>In that case, `childJavaProcess` property will be returned, but `stdout` and `stderr` may be empty, except if an error is triggered at command execution | `false` | `true`
68+
| [detached](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) | If set to true, node will not wait for the java command to be completed.<br/>In that case, `childJavaProcess` property will be returned, but `stdout` and `stderr` may be empty, except if an error is triggered at command execution | `false` | `true`
6969
| [stdoutEncoding](https://nodejs.org/api/stream.html#readablesetencodingencoding) | Adds control on spawn process stdout | `utf8` | `ucs2` |
7070
| waitForErrorMs | If detached is true, number of milliseconds to wait to detect an error before exiting JavaCaller run | `500` | `2000` |
7171
| [cwd](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) | You can override cwd of spawn called by JavaCaller runner | `process.cwd()` | `some/other/cwd/folder` |
7272
| javaArgs | List of arguments for JVM only, not the JAR or the class | `[]` | `['--add-opens=java.base/java.lang=ALL-UNNAMED']` |
7373
| [windowsVerbatimArguments](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) | No quoting or escaping of arguments is done on Windows. Ignored on Unix. This is set to true automatically when shell is specified and is CMD. | `true` | `false` |
7474
| [windowless](https://docs.oracle.com/en/java/javase/17/docs/specs/man/java.html#:~:text=main()%20method.-,javaw,information%20if%20a%20launch%20fails.) | If windowless is true, JavaCaller calls javaw instead of java to not create any windows, useful when using detached on Windows. Ignored on Unix. | false | true
75+
| [timeout](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) | In milliseconds the maximum amount of time the process is allowed to run. | `undefined` | `1000`
76+
| [killSignal](https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) | The signal value to be used when the spawned process will be killed by timeout or abort signal. | `SIGTERM` | `SIGINT`
7577

7678
## Examples
7779

lib/index.d.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,20 @@ export interface JavaCallerRunOptions {
117117
* @default false
118118
*/
119119
windowless?: boolean;
120+
121+
/**
122+
* The number of milliseconds to wait before the Java process will time out. When this occurs,
123+
* killSignal will ben
124+
* @default undefined
125+
*/
126+
timeout?: number;
127+
128+
/**
129+
* If windowless is true, JavaCaller calls javaw instead of java to not create any windows,
130+
* useful when using detached on Windows. Ignored on Unix.
131+
* @default "SIGTERM"
132+
*/
133+
killSignal?: number | NodeJS.Signals;
120134
}
121135

122136
/**

lib/java-caller.js

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,14 @@ class JavaCaller {
6969
* Runs java command of a JavaCaller instance
7070
* @param {string[]} [userArguments] - Java command line arguments
7171
* @param {object} [runOptions] - Run options
72-
* @param {boolean} [runOptions.detached = false] - If set to true, node will node wait for the java command to be completed. In that case, childJavaProcess property will be returned, but stdout and stderr may be empty
72+
* @param {boolean} [runOptions.detached = false] - If set to true, node will not wait for the java command to be completed. In that case, childJavaProcess property will be returned, but stdout and stderr may be empty
7373
* @param {string} [runOptions.stdoutEncoding = 'utf8'] - Adds control on spawn process stdout
7474
* @param {number} [runOptions.waitForErrorMs = 500] - If detached is true, number of milliseconds to wait to detect an error before exiting JavaCaller run
7575
* @param {string} [runOptions.cwd = .] - You can override cwd of spawn called by JavaCaller runner
7676
* @param {string} [runOptions.javaArgs = []] - You can override cwd of spawn called by JavaCaller runner
7777
* @param {string} [runOptions.windowsVerbatimArguments = true] - No quoting or escaping of arguments is done on Windows. Ignored on Unix. This is set to true automatically when shell is specified and is CMD.
78+
* @param {number} [runOptions.timeout] - In milliseconds the maximum amount of time the process is allowed to run
79+
* @param {NodeJS.Signals | number} [runOptions.killSignal = "SIGTERM"] - The signal value to be used when the spawned process will be killed by timeout or abort signal.
7880
* @return {Promise<{status:number, stdout:string, stderr:string, childJavaProcess:ChildProcess}>} - Command result (status, stdout, stderr, childJavaProcess)
7981
*/
8082
async run(userArguments, runOptions = {}) {
@@ -84,6 +86,7 @@ class JavaCaller {
8486
runOptions.stdoutEncoding = typeof runOptions.stdoutEncoding === "undefined" ? "utf8" : runOptions.stdoutEncoding;
8587
runOptions.windowsVerbatimArguments = typeof runOptions.windowsVerbatimArguments === "undefined" ? true : runOptions.windowsVerbatimArguments;
8688
runOptions.windowless = typeof runOptions.windowless === "undefined" ? false : os.platform() !== "win32" ? false : runOptions.windowless;
89+
runOptions.killSignal = typeof runOptions.killSignal === "undefined" ? "SIGTERM" : runOptions.killSignal;
8790
this.commandJavaArgs = (runOptions.javaArgs || []).concat(this.additionalJavaArgs);
8891

8992
let javaExe = runOptions.windowless ? this.javaExecutableWindowless : this.javaExecutable;
@@ -97,10 +100,13 @@ class JavaCaller {
97100

98101
const javaExeToUse = this.javaExecutableFromNodeJavaCaller ?? javaExe;
99102
const classPathStr = this.buildClasspathStr();
100-
const javaArgs = this.buildArguments(classPathStr, (userArguments || []).concat(this.commandJavaArgs));
103+
const javaArgs = this.buildArguments(classPathStr, (userArguments || []).concat(this.commandJavaArgs), runOptions.windowsVerbatimArguments);
101104
let stdout = "";
102105
let stderr = "";
103106
let child;
107+
let timeoutId;
108+
let killedByTimeout = false;
109+
104110
const prom = new Promise((resolve) => {
105111
// Spawn java command line
106112
debug(`Java command: ${javaExeToUse} ${javaArgs.join(" ")}`);
@@ -117,6 +123,19 @@ class JavaCaller {
117123
}
118124
child = spawn(javaExeToUse, javaArgs, spawnOptions);
119125

126+
if (runOptions.timeout) {
127+
timeoutId = setTimeout(() => {
128+
if (!child.killed) {
129+
try {
130+
child.kill(runOptions.killSignal);
131+
killedByTimeout = true;
132+
} catch (err) {
133+
stderr += `Failed to kill process after ${runOptions.timeout}ms: ${err.message}`;
134+
}
135+
}
136+
}, runOptions.timeout);
137+
}
138+
120139
// Gather stdout and stderr if they must be returned
121140
if (spawnOptions.stdio === "pipe") {
122141
child.stdout.setEncoding(`${runOptions.stdoutEncoding}`);
@@ -132,12 +151,28 @@ class JavaCaller {
132151
child.on("error", (data) => {
133152
this.status = 666;
134153
stderr += "Java spawn error: " + data;
154+
155+
if (timeoutId) {
156+
clearTimeout(timeoutId);
157+
}
158+
135159
resolve();
136160
});
137161

138162
// Catch status code
139163
child.on("close", (code) => {
140-
this.status = code;
164+
if (timeoutId) {
165+
clearTimeout(timeoutId);
166+
}
167+
168+
if (killedByTimeout) {
169+
// Process was terminated because of the timeout
170+
this.status = 666;
171+
stderr += `Process timed out with ${runOptions.killSignal} after ${runOptions.timeout}ms.`;
172+
} else {
173+
this.status = code;
174+
}
175+
141176
resolve();
142177
});
143178

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
],
1010
"scripts": {
1111
"lint:fix": "eslint **/*.js --fix && prettier --write \"./lib/**/*.{js,jsx,json}\" --tab-width 4 --print-width 150",
12-
"java:compile": "javac -d test/java/dist -source 8 -target 1.8 test/java/src/com/nvuillam/javacaller/JavaCallerTester.java",
12+
"java:compile": "javac -d test/java/dist --release 8 test/java/src/com/nvuillam/javacaller/JavaCallerTester.java",
1313
"java:jar": "cd test/java/dist && jar -cvfm ./../jar/JavaCallerTester.jar ./../jar/manifest/Manifest.txt com/nvuillam/javacaller/*.class && jar -cvfm ./../jar/JavaCallerTesterRunnable.jar ./../jar/manifest-runnable/Manifest.txt com/nvuillam/javacaller/*.class",
1414
"test": "mocha \"test/**/*.test.js\"",
1515
"test:coverage": "nyc npm run test",
@@ -78,4 +78,4 @@
7878
],
7979
"all": true
8080
}
81-
}
81+
}

test/java-caller.test.js

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,21 @@ describe("Call with classes", () => {
3232
classPath: 'test/java/dist',
3333
mainClass: 'com.nvuillam.javacaller.JavaCallerTester'
3434
});
35+
36+
// JavaCallerTester will sleep for 1000 ms
37+
// After waitForErrorMs (500 ms), the promise will return
3538
const { status, stdout, stderr, childJavaProcess } = await java.run(['--sleep'], { detached: true });
36-
childJavaProcess.kill('SIGINT');
37-
checkStatus(0, status, stdout, stderr);
39+
40+
// Java process is still running
41+
checkStatus(null, status, stdout, stderr);
42+
43+
return new Promise(resolve => {
44+
// Java process has finished executing and the exit code can be read
45+
childJavaProcess.on('exit', () => {
46+
checkStatus(0, childJavaProcess.exitCode, stdout, stderr);
47+
resolve();
48+
});
49+
});
3850
});
3951

4052
it("should call JavaCallerTester.class using javaw", async () => {
@@ -187,4 +199,38 @@ describe("Call with classes", () => {
187199
checkStatus(0, status, stdout, stderr);
188200
checkStdOutIncludes(`JavaCallerTester is called !`, stdout, stderr);
189201
});
202+
203+
it("should terminate once timeout is reached", async () => {
204+
const java = new JavaCaller({
205+
classPath: 'test/java/dist',
206+
mainClass: 'com.nvuillam.javacaller.JavaCallerTester'
207+
});
208+
const { status, stdout, stderr } = await java.run(["--sleep"], { timeout: 500 });
209+
210+
checkStatus(666, status, stdout, stderr);
211+
checkStdErrIncludes(`timed out`, stdout, stderr);
212+
});
213+
214+
it("should not terminate if process finished before timeout is reached", async () => {
215+
const java = new JavaCaller({
216+
classPath: 'test/java/dist',
217+
mainClass: 'com.nvuillam.javacaller.JavaCallerTester'
218+
});
219+
const { status, stdout, stderr } = await java.run(["--sleep"], { timeout: 1500 });
220+
221+
checkStatus(0, status, stdout, stderr);
222+
checkStdOutIncludes(`JavaCallerTester is called !`, stdout, stderr);
223+
});
224+
225+
it("should terminate with custom killSignal when timeout is reached", async () => {
226+
const java = new JavaCaller({
227+
classPath: 'test/java/dist',
228+
mainClass: 'com.nvuillam.javacaller.JavaCallerTester'
229+
});
230+
const { status, stdout, stderr } = await java.run(["--sleep"], { timeout: 500, killSignal: "SIGINT" });
231+
232+
checkStatus(666, status, stdout, stderr);
233+
checkStdErrIncludes(`timed out`, stdout, stderr);
234+
checkStdErrIncludes(`SIGINT`, stdout, stderr);
235+
});
190236
});
62 Bytes
Binary file not shown.

test/java/jar/JavaCallerTester.jar

37 Bytes
Binary file not shown.
32 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)