Skip to content

Commit e68d50a

Browse files
committed
Replace ThreadLocal accumulator with cursor-based lookup in DeclarativeRecipe
PR openrewrite#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 03b700b commit e68d50a

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
}
@@ -495,36 +503,37 @@ public final List<Recipe> getRecipeList() {
495503
return recipeList;
496504
}
497505

498-
List<Supplier<TreeVisitor<?, ExecutionContext>>> andPreconditions = new ArrayList<>();
499-
for (Recipe precondition : preconditions) {
500-
andPreconditions.add(() -> orVisitors(precondition));
501-
}
506+
List<Recipe> preconditionsCopy = new ArrayList<>(preconditions);
502507
//noinspection unchecked
503-
PreconditionBellwether bellwether = new PreconditionBellwether(Preconditions.and(andPreconditions.toArray(new Supplier[]{})));
508+
PreconditionBellwether bellwether = new PreconditionBellwether((rootCursor, ctx) -> {
509+
TreeVisitor<?, ExecutionContext>[] visitors = new TreeVisitor[preconditionsCopy.size()];
510+
for (int i = 0; i < preconditionsCopy.size(); i++) {
511+
visitors[i] = orVisitors(preconditionsCopy.get(i), rootCursor, ctx);
512+
}
513+
return Preconditions.and(visitors);
514+
});
504515
List<Recipe> recipeListWithBellwether = new ArrayList<>(recipeList.size() + 1);
505516
recipeListWithBellwether.add(bellwether);
506517
recipeListWithBellwether.addAll(decorateWithPreconditionBellwether(bellwether, recipeList));
507518
return recipeListWithBellwether;
508519
}
509520

510-
private TreeVisitor<?, ExecutionContext> orVisitors(Recipe recipe) {
521+
@SuppressWarnings({"rawtypes", "unchecked"})
522+
private TreeVisitor<?, ExecutionContext> orVisitors(Recipe recipe, Cursor rootCursor, ExecutionContext ctx) {
511523
List<TreeVisitor<?, ExecutionContext>> conditions = new ArrayList<>();
512524
if (recipe instanceof ScanningRecipe) {
513-
//noinspection rawtypes
514525
ScanningRecipe scanning = (ScanningRecipe) recipe;
515-
//noinspection unchecked
516-
Accumulator acc = accumulator.get();
526+
Accumulator acc = getAccumulator(rootCursor, ctx);
517527
conditions.add(scanning.getVisitor(acc != null ? acc.recipeToAccumulator.get(scanning) : null));
518528
} else {
519529
conditions.add(recipe.getVisitor());
520530
}
521531
for (Recipe r : recipe.getRecipeList()) {
522-
conditions.add(orVisitors(r));
532+
conditions.add(orVisitors(r, rootCursor, ctx));
523533
}
524534
if (conditions.size() == 1) {
525535
return conditions.get(0);
526536
}
527-
//noinspection unchecked
528537
return Preconditions.or(conditions.toArray(new TreeVisitor[0]));
529538
}
530539

@@ -607,16 +616,9 @@ private static class LazyLoadedRecipe extends Recipe {
607616
String description = "Recipe that is loaded lazily.";
608617
}
609618

610-
@Override
611-
public void onComplete(ExecutionContext ctx) {
612-
accumulator.remove();
613-
}
614-
615619
@Override
616620
public DeclarativeRecipe clone() {
617-
DeclarativeRecipe cloned = (DeclarativeRecipe) super.clone();
618-
cloned.accumulator = new ThreadLocal<>();
619-
return cloned;
621+
return (DeclarativeRecipe) super.clone();
620622
}
621623

622624
@Override

0 commit comments

Comments
 (0)