Repository navigation
Conversation
waitsForContextCreationBeforeClosingTheHandle sampled the update thread's state twice: once in the wait loop and again in the assertion. A parked StampedLock writer can briefly leave WAITING between samples (for example on a spurious wakeup, before it re-parks), so the second check could fail even though the loop had already seen the thread blocked in initOrUpdateWafHandle. Assert the value the loop observed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: cgcote <5115638+cgcote@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
More details
Preserving the wait loop’s observed blocked state removes the second-sample race while retaining the timeout, reload-completion, and handle-liveness assertions. No reportable regression was identified.
🤖 Bits Code Review · Commit fccad2c
What Does This Do
Fixes a test-only race in
WAFModuleHandleReloadRaceTest.waitsForContextCreationBeforeClosingTheHandle. The test now asserts the "blocked ininitOrUpdateWafHandle" state that its wait loop actually observed. It no longer samples the update thread's state a second time. Production code is unchanged.Motivation
Flaky Test Management reports this test failing about 0.35% of the time on JDK 8, IBM8 and Semeru17 (amd64 and arm64). It failed at the
assertTrue(blockedInHandleReplacement(updateThread))line on master and in the merge queue, costing about 7.4h of CI across 7 pipelines.The wait loop exits as soon as
blockedInHandleReplacement(updateThread)returns true. The assertion then called the same method again, which re-readsThread.getState()and the stack. The update thread is parked inStampedLock.writeLock()while the evaluation thread holds the read lock. It can briefly be non-WAITING, for example after a spurious wakeup and before it re-parks. A sample taken in that window fails the test, even though the loop had already seen the thread blocked andupdate.isDone()is still false. The locking inWAFModule.initOrUpdateWafHandleis not at fault.Additional Notes
blockedstays false and the test fails. All other assertions are unchanged, including the checks that the reload hasn't finished and the handle stays online during context creation.WAITING. Over 300 repetitions each, the old re-sampling assertion failed 141 times and the new assertion passed every time.WAFModuleHandleReloadRaceTestpass locally on JDK 21. I couldn't reproduce the original flake locally, since it fails only about 0.35% of the time on other JVMs.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
🤖 Generated with Claude Code (link removed)
PR by Bits - View session in Datadog • Open in Slack
Comment @DataDog to request changes