You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Collect garbage before the java-web command exits, which avoids a hang in node's exit teardown - #44
java/math/BigInteger/BitLengthOverflow.java timed out at 600 s in about one CI run in three, so #40 took it out of passing.txt. The test is fine. The java-web process hangs while exiting.
On GitHub's x64 runners, running the test four at a time as a CI shard does:
About 40% of runs printed both expected lines (Overflow reported by ArithmeticException, as expected), so main had returned, and then the process never exited.
A hung process had three threads left, node-MainThread, node-V8Worker and SignalInspector, all asleep in futex_do_wait, and did not answer a diagnostic report signal. It is stuck in node's own exit teardown, after process.exit. The test leaves several hundred MB of BigInteger arrays (array buffers) alive at exit.
It never happened on a small ARM board.
The java-web command now runs a full collection just before process.exit, so node's teardown does not have to free those buffers itself. Same runners, 20 runs each:
Variant
Hung
As on master
5 of 20 (8 of 20 in an earlier run)
gc exposed at node startup instead of at runtime
6 of 20
Collection before process.exit (this PR)
0 of 20
node's --trace-exit (for comparison)
0 of 40
The collection only runs in the command's exit path. Library use (runNode, startLibrary) is unchanged. BitLengthOverflow goes back into passing.txt and times.tsv.
Checks:
Exit codes still come through: System.exit(0), 1 and 2.
pnpm typecheck passes. CI runs every OpenJDK test, since the PR changes src/node.ts.
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
Improvements
Successful CLI runs now perform available runtime cleanup before returning their exit status. This may help reduce leftover resources after a command completes.
Error handling remains unchanged: failures are reported and the command exits with a failure status.
The CLI invokes globalThis.gc when available after successful execution and before exiting with the run’s exit code. The jtreg passing-test list now includes BitLengthOverflow.java.
Changes: CLI exit and jtreg test eligibility
Layer / File(s)
Summary
Successful CLI exit cleanup src/node.ts
After runNode resolves, the CLI calls globalThis.gc when available, then exits with the returned code. The error path is unchanged.
Passing-test list update test/jtreg/passing.txt
The passing-test list adds java/math/BigInteger/BitLengthOverflow.java.
Issue 42 is a release checklist that identifies this work as a release prerequisite.
Possibly related PRs
Project516/java-web#33: The launcher calls the main function whose successful-exit behavior changes here.
Project516/java-web#9: This PR exposes globalThis.gc, which the CLI now calls before a successful exit.
Project516/java-web#37: This PR reports a timeout in the same jtreg test that this change adds to the passing-test list.
Suggested reviewers:project516-review-bot
Merge Risk: Minimal · up to 1415a
The exit cleanup is reachable through the supported CLI path, and no actionable merge risk remains.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly describes the main change: collecting garbage before the java-web command exits to avoid a Node exit teardown hang.
Description check
The description explains the intermittent hang, the pre-exit garbage collection change, the restored test, validation results, and unchanged library behavior.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Simplify code
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
LanguageTool
test/jtreg/passing.txt
LanguageTool could not check every text chunk; analysis is incomplete
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the closing run, And calls the GC before it’s done. One BigInteger test joins the queue, The passing list now names it too. Soft paws applaud the tidy end.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 9f31cafa-defc-47a4-8b98-be30a044467c
Commits
Reviewing files that changed from the base of the PR and between 1a40f3e and 1415aa6.
Files ignored due to path filters: 1
test/jtreg/times.tsv is excluded by !**/*.tsv
Files selected for processing: 2
src/node.ts
test/jtreg/passing.txt
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
The java-web command now forces garbage collection via globalThis.gc?.() immediately before process.exit, so node's exit teardown does not have to free large array buffers itself. This eliminates the hang observed in CI on x64 runners.
review-bot, model nvidia/nemotron-3.5-lightning:free, verdict approve
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
java/math/BigInteger/BitLengthOverflow.javatimed out at 600 s in about one CI run in three, so #40 took it out ofpassing.txt. The test is fine. Thejava-webprocess hangs while exiting.On GitHub's x64 runners, running the test four at a time as a CI shard does:
Overflow reported by ArithmeticException, as expected), somainhad returned, and then the process never exited.node-MainThread,node-V8WorkerandSignalInspector, all asleep infutex_do_wait, and did not answer a diagnostic report signal. It is stuck in node's own exit teardown, afterprocess.exit. The test leaves several hundred MB ofBigIntegerarrays (array buffers) alive at exit.The
java-webcommand now runs a full collection just beforeprocess.exit, so node's teardown does not have to free those buffers itself. Same runners, 20 runs each:gcexposed at node startup instead of at runtimeprocess.exit(this PR)--trace-exit(for comparison)The collection only runs in the command's exit path. Library use (
runNode,startLibrary) is unchanged.BitLengthOverflowgoes back intopassing.txtandtimes.tsv.Checks:
System.exit(0),1and2.pnpm typecheckpasses. CI runs every OpenJDK test, since the PR changessrc/node.ts.