Skip to content

Commit 601bcea

Browse files
committed
Replace ThreadLocal accumulator with cursor-based lookup in DeclarativeRecipe
PR #6810 changed DeclarativeRecipe.accumulator from a plain field to ThreadLocal to fix concurrent recipe runs overwriting each other's accumulator. However, ThreadLocal values are lost when execution resumes on a different thread after yielding. The accumulator is already stored on the rootCursor by ScanningRecipe.getAccumulator(), which is per-run and survives thread switches. This change removes the redundant ThreadLocal storage and instead resolves the accumulator from the rootCursor at visit time. The PreconditionBellwether now defers precondition resolution to visit time (when the rootCursor is available) rather than eagerly resolving in a field initializer.
1 parent 9a2690b commit 601bcea

1 file changed

Lines changed: 30 additions & 28 deletions

File tree

rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.time.Duration;
2727
import java.util.*;
2828
import java.util.function.Function;
29-
import java.util.function.Supplier;
3029

3130
import static java.util.Collections.emptyList;
3231
import static org.openrewrite.Validated.invalid;
@@ -161,16 +160,12 @@ private void initializeDeclarativeRecipe(DeclarativeRecipe declarativeRecipe, St
161160
}
162161
}
163162

164-
@JsonIgnore
165-
private transient ThreadLocal<Accumulator> accumulator = new ThreadLocal<>();
166-
167163
@Override
168164
public Accumulator getInitialValue(ExecutionContext ctx) {
169165
Accumulator acc = new Accumulator();
170166
for (Recipe precondition : preconditions) {
171167
registerNestedScanningRecipes(precondition, acc, ctx);
172168
}
173-
accumulator.set(acc);
174169
return acc;
175170
}
176171

@@ -212,6 +207,11 @@ public static class Accumulator {
212207
Map<Recipe, Object> recipeToAccumulator = new HashMap<>();
213208
}
214209

210+
@FunctionalInterface
211+
interface PreconditionResolver {
212+
TreeVisitor<?, ExecutionContext> resolve(Cursor rootCursor, ExecutionContext ctx);
213+
}
214+
215215
@Value
216216
@EqualsAndHashCode(callSuper = false)
217217
@RequiredArgsConstructor
@@ -222,27 +222,35 @@ static class PreconditionBellwether extends Recipe {
222222
String description = "Evaluates a precondition and makes that result available to the preconditions of other recipes. " +
223223
"\"bellwether\", noun - One that serves as a leader or as a leading indicator of future trends.";
224224

225-
Supplier<TreeVisitor<?, ExecutionContext>> precondition;
225+
PreconditionResolver precondition;
226226

227227
@NonFinal
228228
transient boolean preconditionApplicable;
229229

230230
@Override
231231
public TreeVisitor<?, ExecutionContext> getVisitor() {
232232
return new TreeVisitor<Tree, ExecutionContext>() {
233-
final TreeVisitor<?, ExecutionContext> p = precondition.get();
233+
@Nullable
234+
TreeVisitor<?, ExecutionContext> p;
234235

235236
@Override
236237
public boolean isAcceptable(SourceFile sourceFile, ExecutionContext ctx) {
237-
return p.isAcceptable(sourceFile, ctx);
238+
return resolve(ctx).isAcceptable(sourceFile, ctx);
238239
}
239240

240241
@Override
241242
public @Nullable Tree visit(@Nullable Tree tree, ExecutionContext ctx) {
242-
Tree t = p.visit(tree, ctx);
243+
Tree t = resolve(ctx).visit(tree, ctx);
243244
preconditionApplicable = t != tree;
244245
return tree;
245246
}
247+
248+
private TreeVisitor<?, ExecutionContext> resolve(ExecutionContext ctx) {
249+
if (p == null) {
250+
p = precondition.resolve(getCursor(), ctx);
251+
}
252+
return p;
253+
}
246254
};
247255
}
248256
}
@@ -490,36 +498,37 @@ public final List<Recipe> getRecipeList() {
490498
return recipeList;
491499
}
492500

493-
List<Supplier<TreeVisitor<?, ExecutionContext>>> andPreconditions = new ArrayList<>();
494-
for (Recipe precondition : preconditions) {
495-
andPreconditions.add(() -> orVisitors(precondition));
496-
}
501+
List<Recipe> preconditionsCopy = new ArrayList<>(preconditions);
497502
//noinspection unchecked
498-
PreconditionBellwether bellwether = new PreconditionBellwether(Preconditions.and(andPreconditions.toArray(new Supplier[]{})));
503+
PreconditionBellwether bellwether = new PreconditionBellwether((rootCursor, ctx) -> {
504+
TreeVisitor<?, ExecutionContext>[] visitors = new TreeVisitor[preconditionsCopy.size()];
505+
for (int i = 0; i < preconditionsCopy.size(); i++) {
506+
visitors[i] = orVisitors(preconditionsCopy.get(i), rootCursor, ctx);
507+
}
508+
return Preconditions.and(visitors);
509+
});
499510
List<Recipe> recipeListWithBellwether = new ArrayList<>(recipeList.size() + 1);
500511
recipeListWithBellwether.add(bellwether);
501512
recipeListWithBellwether.addAll(decorateWithPreconditionBellwether(bellwether, recipeList));
502513
return recipeListWithBellwether;
503514
}
504515

505-
private TreeVisitor<?, ExecutionContext> orVisitors(Recipe recipe) {
516+
@SuppressWarnings({"rawtypes", "unchecked"})
517+
private TreeVisitor<?, ExecutionContext> orVisitors(Recipe recipe, Cursor rootCursor, ExecutionContext ctx) {
506518
List<TreeVisitor<?, ExecutionContext>> conditions = new ArrayList<>();
507519
if (recipe instanceof ScanningRecipe) {
508-
//noinspection rawtypes
509520
ScanningRecipe scanning = (ScanningRecipe) recipe;
510-
//noinspection unchecked
511-
Accumulator acc = accumulator.get();
521+
Accumulator acc = getAccumulator(rootCursor, ctx);
512522
conditions.add(scanning.getVisitor(acc != null ? acc.recipeToAccumulator.get(scanning) : null));
513523
} else {
514524
conditions.add(recipe.getVisitor());
515525
}
516526
for (Recipe r : recipe.getRecipeList()) {
517-
conditions.add(orVisitors(r));
527+
conditions.add(orVisitors(r, rootCursor, ctx));
518528
}
519529
if (conditions.size() == 1) {
520530
return conditions.get(0);
521531
}
522-
//noinspection unchecked
523532
return Preconditions.or(conditions.toArray(new TreeVisitor[0]));
524533
}
525534

@@ -602,16 +611,9 @@ private static class LazyLoadedRecipe extends Recipe {
602611
String description = "Recipe that is loaded lazily.";
603612
}
604613

605-
@Override
606-
public void onComplete(ExecutionContext ctx) {
607-
accumulator.remove();
608-
}
609-
610614
@Override
611615
public DeclarativeRecipe clone() {
612-
DeclarativeRecipe cloned = (DeclarativeRecipe) super.clone();
613-
cloned.accumulator = new ThreadLocal<>();
614-
return cloned;
616+
return (DeclarativeRecipe) super.clone();
615617
}
616618

617619
@Override

0 commit comments

Comments
 (0)