Repository navigation
test(runner): analyse a state registered but not yet written to (#1212) - #1216
Merged
Merged
Conversation
…t lambdas (#1172) Only forEach's action carried the run to the common pool's workers (#1156), so a race inside a parallel map lambda was seen on the caller's own elements alone. A lambda given to Stream.map, filter or peek now waits in a slot. The woven terminal operation (forEach, forEachOrdered, collect, reduce, min, max, the matches, toList, toArray, count, the finds) lends the run to every slot of its pipeline while it runs, and its own lambdas run like forEach's action. A pipeline ended by an unwoven terminal such as iterator() lends nothing: with no join to order the caller after the helpers, lending would report the read of a result as a race. A combiner, a reduce operator or a comparator merges what other subtasks built, after the JDK's fork and join, which the agent does not see. Each element now publishes its thread's clock, the caller's included, and those three receive every clock published so far before they run, so a correct three-argument collect stays silent. Not followed yet: a primitive stream's map and filter, a stage behind an unwoven one (sorted, distinct, limit, mapToInt) and a Collector's own functions; documented in the limitations. Tests first: the pipeline and terminal attribution tests, then the two combiner-ordering tests, which reported a correct collect and reduce as races until the combine rule. Corpus: four new idioms, the two broken ones fire PROMPT/HIGH and the two correct ones stay silent, 0 false positives, every gate green. Library suite 3921 green, agent suite green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…1172) Corpus Eval went red on JDK 25 and 26: SharedCollectionDetector reported the correct three-argument collect (idiom_parallelCollect_containerPerSubtask) as concurrent ArrayList writes. The corpus JVM had started about 18,000 workers by then, and a happens-before clock keeps 4,096 threads (#840), dropping the lowest ids first. The common pool's threads are started early and keep working, so their ids are below every ended worker's, and they were the first entries dropped. A combine on one pool thread received the clock of the pool thread that filled the container it merges and dropped that entry in the same join, so the merge looked unordered. The same loss hit the caller's read after a parallel forEach (#1156) once a suite had met that many threads. Past the cap, entries of threads that have ended now go first, lowest ids first, then the lowest ids of running ones. Any choice still only loses an edge, never adds one. The model keeps a weak id-to-thread map so the over-cap join can ask whether a thread is still running. Verified: the new AgentParallelStreamHooksTest case (a caller that met 4,104 threads, then a correct collect) failed on JDK 26 before the fix, and its full clock also turned three existing ordering tests red in the same JVM; HappensBeforeTest.theCapDropsEndedThreadsFirst failed before the fix. After it: library suite 3923 green on JDK 26, static gates green on JDK 21, corpus eval on JDK 26 green in every lane (idiom lane 82/82). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test Suite (21) failed in DetectorAnalysisUnderConcurrentWritesTest: CompletableFutureObtrudeDetector.analyze() threw a NullPointerException while another thread was still recording. recordObtrude creates the future's state before it sets lastObtrudedByThread, and analyze() passed that null to Map.of, which threw. ifIssue then dropped the run's whole report. analyze() now reads the field once and skips a state whose thread is not set yet. The analysis at the end of the run reports it. Verified on JDK 21: the new anObtrudeStillBeingRecordedDoesNotBreakAnalysis failed before the fix and passes after it, with DetectorAnalysisUnderConcurrentWritesTest green; static gates green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
) Corpus Eval stayed red on JDK 25 and 26: SharedCollectionDetector still reported the correct three-argument collect (idiom_parallelCollect_containerPerSubtask) after the clock-cap fix. From JDK 24 the common pool's workers are innocuous: each time one goes idle and wakes up again it clears its thread-locals, often partway through one parallel stream. The thread's clock lives in the CLOCKS thread-local, so the next access started a fresh clock, whose own count began again and which had forgotten everything the thread acquired. A combine on that thread then merged a container it had filled itself and read the merge as unordered. A CI trace showed it: worker-3's own count was 380 when it wrote the list, while the clocks it published carried 126 for worker-1, whose clock was at 252. JDK 21 does not clear them, which is why it never failed. start() now returns the clock THREADS already holds for the thread, so the same thread keeps the same clock. Verified on JDK 25: the new aClearedThreadLocalKeepsTheClock failed before the fix and passes after it; the full library build with all gates (3925 + 113 + 2 tests) and corpus-eval (139 tests) are green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ambdas # Conflicts: # docs/CHANGELOG.md
The timing test for #1150 runs writers and an analyser for 120 ms and catches a detector that iterates a plain collection, but it rarely catches a record path that publishes its state before writing to it: that window is a few instructions. CompletableFutureObtrudeDetector had exactly that defect and passed the timing test on every leg of #1203, then failed on main's JDK 26 leg. The new test calls AbstractInstanceDetector.stateFor directly for every per-instance detector that registers through newState, which leaves the state as another thread's analysis sees it between stateFor and the first write, and analyses it. It reaches 54 detectors (floor 50). Verified: on origin/main, which lacks the obtrude fix, it is red 4 of 4 naming only CompletableFutureObtrudeDetector (NullPointerException), while the timing test was green 4 of 4 in the same runs. Stacked on #1209, which carries that fix, both tests are green. Factory-registered and non-instance detectors are not covered: #1215. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stacked on #1209: review the last commit only. Merge #1209 first, with a merge commit. The
CompletableFutureObtrudeDetectorfix this test needs is now on main (#1219), and both branches have merged main.Why
DetectorAnalysisUnderConcurrentWritesTest(#1150) runs writers and an analyser for 120 ms per detector. That catches ananalyze()that iterates a plain collection, because the race window is the whole iteration. It rarely catches a record path that publishes a state before writing to it, because that window is a few instructions.CompletableFutureObtrudeDetectorhas that defect: it passed every leg of #1203, then turned main red on JDK 26 (run 42373466647). That fix landed on main in #1219.What
A second test in the same class. It calls
AbstractInstanceDetector.stateFordirectly for every per-instance detector that registers throughnewState, once named and once unnamed, then runsanalyze(). That leaves each state exactly as another thread's analysis sees it betweenstateForand the record path's first write. Unlike the timing test, the window is open every run.Verified
origin/main, without the fix, the new test was red 4 of 4 and named onlyCompletableFutureObtrudeDetector(NullPointerException). The timing test was green 4 of 4 in the same runs.CompletableFutureObtrudeDetectorTest's 8 tests.Not covered
Factory-registered detectors and the ones outside
AbstractInstanceDetectorstill rely on the timing test only: #1215.Docs
Test-only, with no behaviour change, so no CHANGELOG entry (the obtrude fix's entry is in #1209). The class javadoc explains why there are two tests.
Closes #1212
🤖 Generated with Claude Code