Repository navigation
Keep park, wait and sleep with huge timeouts from waking after 1 ms - #66
Conversation
…ait and sleep with huge timeouts no longer wake early or spin
|
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 4 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. |
![project516-review-bot[bot]](https://avatars.githubusercontent.com/in/4903871?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR fixes a bug where host timers clamp delays > 2^31-1 ms to 1 ms, causing park/wait/sleep with huge timeouts to wake early. The fix re-arms the timer in MAX_DELAY steps. A race condition exists: if an interrupt occurs between a step timer firing and its re-arm callback executing, the wait resolves but the callback still re-arms a stray timer. The test covers the four wait paths and passes with the fix.
review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict request_changes
| // timers fire at once past MAX_DELAY, so a longer wait re-arms | ||
| const arm = (left: number) => { | ||
| const step = Math.min(Math.max(0, left), MAX_DELAY); | ||
| timer = setTimeout(step < left ? () => { t.vm.timers.delete(timer!); arm(left - step); } : done, step); |
There was a problem hiding this comment.
Race condition: if an interrupt wakes the thread (calling done, which sets t.wake = null) after a step timer fires but before its re-arm callback runs, the callback still executes and re-arms a new timer. The wait has already resolved, so this creates a stray timer that will fire later and call done again. Fix: check if (t.wake === null) return; at the start of the re-arm callback before deleting the timer and calling arm.
What this changes
Host timers hold at most 2^31-1 ms (about 24.8 days). A longer delay fires after 1 ms instead.
park,Object.waitandThread.sleepwith a huge timeout, such asLockSupport.parkNanos(Long.MAX_VALUE), therefore woke after 1 ms.parkandwaitreturned early, andsleeplooped, waking every millisecond.interruptibleWait, which all three use, now re-arms the timer in steps of at most that limit.Tests
test/java/LongTimeouts.javablocks four threads inparkNanos,parkUntil,waitandsleepwith huge timeouts and interrupts them after 300 ms. java-web now prints the same as HotSpot. Before,parkandwaitreported an early wake.pnpm typecheckpasses.Several OpenJDK virtual-thread and
java.util.concurrenttests that hung or timed out may have been hitting this. They are not added here; the next sweep will show which ones now pass.