Repository navigation
Fix two flakes in the HeapCap heap-limit test - #53
Conversation
… the heap before HeapCap prints
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 minutes. View limit details
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. |
There was a problem hiding this comment.
The PR fixes two flakes in the HeapCap test. In runtime.ts, the allocation check now requires two consecutive over-cap readings before throwing OOM, addressing a Node.js quirk where array buffers are freed after GC rather than during it. In HeapCap.java, the caught OOM handler clears the heap-retaining list before printing the error message, preventing a second OOM during string allocation. Both changes are targeted and correct.
review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict approve
Fixes
HeapCap, which #50 added and which fails some CI runs (it failed #51'stestcheck on x64).Two separate flakes:
OutOfMemoryErrorthere, so the expected output changed. The program now clears the list before printing. Three HotSpot runs here give the same output each time.OutOfMemoryErroron the allocations after the list was cleared. The check collects and measures again before throwing, but Node frees array buffers after a collection rather than during it, soarrayBuffersread right aftergc()can still count arrays that are already garbage. The VM now throws only when two checks in a row are over the cap. A heap that really is full still throws, one check step (at least 4 MB of allocation) later; a stale reading clears by the next check.Checks:
node src/node.ts -Xmx32m HeapCappassed 12 runs out of 12 here (it failed 1 in 5 before), andpnpm typecheckpasses.