Replace ThreadLocal with cursor-based accumulator in DeclarativeRecipe#7225
Merged
knutwannheden merged 1 commit intomainfrom Apr 1, 2026
Merged
Replace ThreadLocal with cursor-based accumulator in DeclarativeRecipe#7225knutwannheden merged 1 commit intomainfrom
ThreadLocal with cursor-based accumulator in DeclarativeRecipe#7225knutwannheden merged 1 commit intomainfrom
Conversation
…veRecipe When a ScanningRecipe is used as a precondition in a DeclarativeRecipe, the accumulator was stored in a ThreadLocal during the scan phase and read back in orVisitors() during the edit phase. Worker yielding could cause the edit phase to resume on a different thread, where the ThreadLocal was null, resulting in NPE. The fix uses the existing cursor-based accumulator storage mechanism (ScanningRecipe.getAccumulator) which stores accumulators on the root cursor keyed by UUID. This survives across threads since the cursor is passed through RecipeRunCycle boundaries. Changes: - Remove ThreadLocal<Accumulator> field - PreconditionBellwether now lazily resolves precondition visitors in visit() where cursor and ExecutionContext are available - orVisitors() takes Cursor + ExecutionContext parameters and uses getAccumulator(rootCursor, ctx) instead of ThreadLocal - Remove onComplete() that only cleared the ThreadLocal - Simplify clone() which only copied the ThreadLocal Fixes #7159
knutwannheden
changed the title
ThreadLocal with cursor-based accumulator in DeclarativeRecipe
timtebeek
approved these changes
Apr 1, 2026
github-project-automation
bot
moved this from In Progress
to Ready to Review
in OpenRewrite
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
2 participants
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.
Motivation
When a
ScanningRecipeis used as a precondition in aDeclarativeRecipe, the accumulator is stored in aThreadLocalduring the scan phase and read back inorVisitors()during the edit phase. This means the scan and edit phases must run on the same thread — otherwise theThreadLocalis null, resulting in NPE for anyScanningRecipeprecondition that uses its accumulator ingetVisitor().This affects
ModuleHasDependency,RepositoryHasDependency,HasNoJakartaAnnotations, and similar recipes when used as preconditions. The error manifests as all files in a run failing (not intermittent), because once a thread switch occurs between scan and edit phases, the entire edit phase runs on the new thread where theThreadLocalis empty.ScanningRecipealready has a thread-safe mechanism for storing accumulators:getAccumulator(Cursor, ExecutionContext)stores them on the root cursor keyed by UUID (line 94-96). This survives across threads since the cursor is passed throughRecipeRunCycleboundaries.DeclarativeRecipeshould use this same mechanism instead of aThreadLocal.Summary
ThreadLocal<Accumulator>fromDeclarativeRecipePreconditionBellwethernow lazily resolves precondition visitors invisit(), where both the cursor andExecutionContextare availableorVisitors()takesCursor+ExecutionContextand usesgetAccumulator(rootCursor, ctx)— the same UUID-keyed cursor message mechanism thatScanningRecipe.getVisitor()already usesonComplete()(only cleared the ThreadLocal) and simplifyclone()No public API changes.
Test plan
rewrite-core:test— all passrewrite-java-test:test— all passExisting
DeclarativeRecipeTest.scanningPreconditionMetandscanningPreconditionNotMetcover the happy pathFixes DeclarativeRecipe ThreadLocal accumulator causes NPE for ScanningRecipe preconditions in multi-threaded execution #7159