From bb9ec4ad2d5f9dd5a8fec9d0c48516b6153fe2b7 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Wed, 1 Apr 2026 09:41:37 +0200 Subject: [PATCH] Replace ThreadLocal accumulator with cursor-based lookup in DeclarativeRecipe 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 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 --- .../openrewrite/config/DeclarativeRecipe.java | 58 +++++++++---------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java b/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java index fcad720bd22..3d00b38d9f9 100644 --- a/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java +++ b/rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java @@ -26,7 +26,6 @@ import java.time.Duration; import java.util.*; import java.util.function.Function; -import java.util.function.Supplier; import static java.util.Collections.emptyList; import static org.openrewrite.Validated.invalid; @@ -162,16 +161,12 @@ private void initializeDeclarativeRecipe(DeclarativeRecipe declarativeRecipe, St } } - @JsonIgnore - private transient ThreadLocal accumulator = new ThreadLocal<>(); - @Override public Accumulator getInitialValue(ExecutionContext ctx) { Accumulator acc = new Accumulator(); for (Recipe precondition : preconditions) { registerNestedScanningRecipes(precondition, acc, ctx); } - accumulator.set(acc); return acc; } @@ -223,7 +218,7 @@ static class PreconditionBellwether extends Recipe { String description = "Evaluates a precondition and makes that result available to the preconditions of other recipes. " + "\"bellwether\", noun - One that serves as a leader or as a leading indicator of future trends."; - Supplier> precondition; + DeclarativeRecipe declarativeRecipe; @NonFinal transient boolean preconditionApplicable; @@ -231,19 +226,35 @@ static class PreconditionBellwether extends Recipe { @Override public TreeVisitor getVisitor() { return new TreeVisitor() { - final TreeVisitor p = precondition.get(); + @Nullable + TreeVisitor p; @Override public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) { - return p.isAcceptable(sourceFile, ctx); + // p is lazily resolved in visit() where the cursor is available. + // Before that, conservatively accept all source files. + return p == null || p.isAcceptable(sourceFile, ctx); } @Override public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) { - Tree t = p.visit(tree, ctx); + Tree t = resolve(ctx).visit(tree, ctx); preconditionApplicable = t != tree; return tree; } + + private TreeVisitor resolve(ExecutionContext ctx) { + if (p == null) { + Cursor rootCursor = getCursor().getRoot(); + List> andVisitors = new ArrayList<>(); + for (Recipe precondition : declarativeRecipe.preconditions) { + andVisitors.add(declarativeRecipe.orVisitors(precondition, rootCursor, ctx)); + } + //noinspection unchecked + p = Preconditions.and(andVisitors.toArray(new TreeVisitor[0])); + } + return p; + } }; } } @@ -496,36 +507,30 @@ public final List getRecipeList() { return recipeList; } - List>> andPreconditions = new ArrayList<>(); - for (Recipe precondition : preconditions) { - andPreconditions.add(() -> orVisitors(precondition)); - } - //noinspection unchecked - PreconditionBellwether bellwether = new PreconditionBellwether(Preconditions.and(andPreconditions.toArray(new Supplier[]{}))); + PreconditionBellwether bellwether = new PreconditionBellwether(this); List recipeListWithBellwether = new ArrayList<>(recipeList.size() + 1); recipeListWithBellwether.add(bellwether); recipeListWithBellwether.addAll(decorateWithPreconditionBellwether(bellwether, recipeList)); return recipeListWithBellwether; } - private TreeVisitor orVisitors(Recipe recipe) { + @SuppressWarnings({"rawtypes", "unchecked"}) + private TreeVisitor orVisitors(Recipe recipe, Cursor rootCursor, ExecutionContext ctx) { List> conditions = new ArrayList<>(); if (recipe instanceof ScanningRecipe) { - //noinspection rawtypes ScanningRecipe scanning = (ScanningRecipe) recipe; - //noinspection unchecked - Accumulator acc = accumulator.get(); - conditions.add(scanning.getVisitor(acc != null ? acc.recipeToAccumulator.get(scanning) : null)); + Accumulator acc = getAccumulator(rootCursor, ctx); + Object recipeAcc = acc.recipeToAccumulator.get(scanning); + conditions.add(scanning.getVisitor(recipeAcc)); } else { conditions.add(recipe.getVisitor()); } for (Recipe r : recipe.getRecipeList()) { - conditions.add(orVisitors(r)); + conditions.add(orVisitors(r, rootCursor, ctx)); } if (conditions.size() == 1) { return conditions.get(0); } - //noinspection unchecked return Preconditions.or(conditions.toArray(new TreeVisitor[0])); } @@ -608,16 +613,9 @@ private static class LazyLoadedRecipe extends Recipe { String description = "Recipe that is loaded lazily."; } - @Override - public void onComplete(ExecutionContext ctx) { - accumulator.remove(); - } - @Override public DeclarativeRecipe clone() { - DeclarativeRecipe cloned = (DeclarativeRecipe) super.clone(); - cloned.accumulator = new ThreadLocal<>(); - return cloned; + return (DeclarativeRecipe) super.clone(); } @Override