From a7dc018d6252af7b191e3f10a182c9b9e85063e6 Mon Sep 17 00:00:00 2001 From: Marco Ferretti Date: Thu, 4 Jun 2026 16:21:26 +0200 Subject: [PATCH 1/5] Make FakeValuesService expression caches static: share across Fakers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the per-instance REGEXP2SUPPLIER_MAP with a two-level static+instance design: - L1 RECIPE_MAP (static): keyed by CacheKey(String exp, SingletonLocale locale) — no per-Faker references. Stores context-free "recipe" resolvers shared across all Fakers with the same locale. Fixes the memory leak that blocked making this cache static (issue #1263): the old RegExpContext key held a strong reference to the BaseFaker root, keeping every Faker alive indefinitely. - L2 instanceMap (per-instance): stores resolvers pre-bound to this Faker's concrete provider instances for fast repeated calls within the same Faker. ValueResolver gains materialize(ProviderRegistration) and cacheable() to support the two-level contract. New resolver types (ProviderMethodResolver, ChainedCoercedResolver, InstanceMethodResolver, etc.) are context-free at L1 and pre-bound at L2. expression2generex (RgxGen compiled-regex cache) is also made static Adds SharedFakeValuesServiceTest covering concurrent multi-Faker usage and determinism under caching. --- .../datafaker/service/FakeValuesService.java | 208 +++++++++++++----- .../SharedFakeValuesServiceTest.java | 86 ++++++++ 2 files changed, 241 insertions(+), 53 deletions(-) create mode 100644 src/test/java/net/datafaker/SharedFakeValuesServiceTest.java diff --git a/src/main/java/net/datafaker/service/FakeValuesService.java b/src/main/java/net/datafaker/service/FakeValuesService.java index 4c74b0377..3926fd07c 100644 --- a/src/main/java/net/datafaker/service/FakeValuesService.java +++ b/src/main/java/net/datafaker/service/FakeValuesService.java @@ -66,7 +66,7 @@ public class FakeValuesService { private static final JsonTransformer JSON_TRANSFORMER = JsonTransformer.builder().build(); - private final Map expression2generex = new CopyOnWriteMap<>(WeakHashMap::new); + private static final Map expression2generex = new CopyOnWriteMap<>(WeakHashMap::new); private final CopyOnWriteMap> key2Expression = new CopyOnWriteMap<>(IdentityHashMap::new); private static final Map ARGS_2_SPLITTED_ARGS = new CopyOnWriteMap<>(WeakHashMap::new); @@ -81,7 +81,12 @@ public class FakeValuesService { private static final Map EXPRESSION_2_SPLITTED = new CopyOnWriteMap<>(WeakHashMap::new); - private final Map REGEXP2SUPPLIER_MAP = new CopyOnWriteMap<>(HashMap::new); + /** L1: static recipe cache — context-free resolvers shared across all Fakers with same locale. */ + private static final Map RECIPE_MAP = new CopyOnWriteMap<>(HashMap::new); + + /** L2: per-instance materialized cache — resolvers pre-bound to this Faker's providers for fast repeated calls. */ + private final Map instanceMap = new CopyOnWriteMap<>(HashMap::new); + public void updateFakeValuesInterfaceMap(List locales) { for (final SingletonLocale l : locales) { fakeValuesInterfaceMap.computeIfAbsent(l, this::getCachedFakeValue); @@ -165,23 +170,21 @@ public String fetchString(String key, FakerContext context) { return (String) fetch(key, context); } - private class SafeFetchResolver implements ValueResolver { + private static class SafeFetchResolver implements ValueResolver { private final String simpleDirective; - private final FakerContext context; - private SafeFetchResolver(String simpleDirective, FakerContext context) { + private SafeFetchResolver(String simpleDirective) { this.simpleDirective = simpleDirective; - this.context = context; } @Override - public Object resolve() { - return safeFetch(simpleDirective, context, null); + public Object resolve(ProviderRegistration root, FakerContext context) { + return root.fakeValuesService().safeFetch(simpleDirective, context, null); } @Override public String toString() { - return "%s[simpleDirective=%s, context=%s]".formatted(getClass().getSimpleName(), simpleDirective, context); + return "%s[simpleDirective=%s]".formatted(getClass().getSimpleName(), simpleDirective); } } @@ -593,20 +596,35 @@ protected String resolveExpression(String expression, Object current, ProviderRe } continue; } - final RegExpContext regExpContext = new RegExpContext(expr, root, context); - final ValueResolver val = REGEXP2SUPPLIER_MAP.get(regExpContext); final Object resolved; - if (val != null) { - resolved = val.resolve(); + // L2: per-instance hit — fast, provider already bound + final ValueResolver fast = instanceMap.get(expr); + if (fast != null) { + resolved = fast.resolve(root, context); } else { - int j = 0; - final int length = expr.length(); - while (j < length && !Character.isWhitespace(expr.charAt(j))) j++; - String directive = expr.substring(0, j); - while (j < length && Character.isWhitespace(expr.charAt(j))) j++; - final String arguments = j == length ? "" : expr.substring(j); - final String[] args = splitArguments(arguments); - resolved = resExp(directive, args, current, root, context, regExpContext); + final CacheKey cacheKey = new CacheKey(expr, context.getSingletonLocale()); + // L1: static recipe hit — materialize once, store in L2 + final ValueResolver recipe = RECIPE_MAP.get(cacheKey); + if (recipe != null) { + final ValueResolver materialized = recipe.materialize(root); + instanceMap.put(expr, materialized); + resolved = materialized.resolve(root, context); + } else { + // Both miss: full discovery + int j = 0; + final int length = expr.length(); + while (j < length && !Character.isWhitespace(expr.charAt(j))) j++; + String directive = expr.substring(0, j); + while (j < length && Character.isWhitespace(expr.charAt(j))) j++; + final String arguments = j == length ? "" : expr.substring(j); + final String[] args = splitArguments(arguments); + resolved = resExp(directive, args, current, root, context, cacheKey); + // resExp stored recipe in RECIPE_MAP if cacheable; materialize for L2 + final ValueResolver stored = RECIPE_MAP.get(cacheKey); + if (stored != null) { + instanceMap.put(expr, stored.materialize(root)); + } + } } if (resolved == null) { throw new RuntimeException("Unable to resolve #{" + expr + "} directive for FakerContext " + context + "."); @@ -681,12 +699,12 @@ private String[] splitExpressions(String expression, int length) { }); } - private Object resExp(String directive, String[] args, Object current, ProviderRegistration root, FakerContext context, RegExpContext regExpContext) { + private Object resExp(String directive, String[] args, Object current, ProviderRegistration root, FakerContext context, CacheKey cacheKey) { Object res = resolveExpression(directive, args, current, root, context); - LOG.fine(() -> "resExp(%s [%s]) current: %s, root: %s, context: %s, regExpContext: %s -> res: %s".formatted(directive, Arrays.toString(args), current, root, context, regExpContext, res)); + LOG.fine(() -> "resExp(%s [%s]) current: %s, root: %s, context: %s, cacheKey: %s -> res: %s".formatted(directive, Arrays.toString(args), current, root, context, cacheKey, res)); if (res instanceof CharSequence) { if (((CharSequence) res).isEmpty()) { - REGEXP2SUPPLIER_MAP.put(regExpContext, EMPTY_STRING); + RECIPE_MAP.put(cacheKey, EMPTY_STRING); } return res; } @@ -696,11 +714,13 @@ private Object resExp(String directive, String[] args, Object current, ProviderR Object valueResolver = it.next(); Object value; if (valueResolver instanceof ValueResolver resolver) { - value = resolver.resolve(); + value = resolver.resolve(root, context); if (value == null) { it.remove(); } else { - REGEXP2SUPPLIER_MAP.put(regExpContext, resolver); + if (resolver.cacheable()) { + RECIPE_MAP.put(cacheKey, resolver); + } return value; } } @@ -733,11 +753,11 @@ private Object resolveExpression(String directive, String[] args, Object current if (current instanceof AbstractProvider) { final Method method = BaseFaker.getMethod((AbstractProvider) current, directive); if (method != null) { - res.add(new MethodResolver(method, current, args)); + res.add(new ProviderMethodResolver(current.getClass().getSimpleName(), method, args)); return res; } } - res.add(resolveFromMethodOn(current, directive, args)); + res.add(resolveFromMethodOn(current, directive, args, root)); } if (dotIndex > 0) { String providerClassName = directive.substring(0, dotIndex); @@ -745,7 +765,7 @@ private Object resolveExpression(String directive, String[] args, Object current AbstractProvider ap = root.getProvider(providerClassName); Method method = ap == null ? null : ObjectMethods.getMethodByName(ap, methodName); if (method != null) { - res.add(new MethodResolver(method, ap, args)); + res.add(new ProviderMethodResolver(providerClassName, method, args)); return res; } } @@ -758,12 +778,12 @@ private Object resolveExpression(String directive, String[] args, Object current // car.wheel will be looked up in the YAML file. // It's only "simple" if there aren't args if (args.length == 0) { - res.add(new SafeFetchResolver(simpleDirective, context)); + res.add(new SafeFetchResolver(simpleDirective)); } // resolve method references on faker object like #{regexify '[a-z]'} if (dotIndex == -1 && root != null && (current == null || root.getClass() != current.getClass())) { - res.add(resolveFromMethodOn(root, directive, args)); + res.add(resolveFromMethodOn(root, directive, args, root)); } // Resolve Faker Object method references like #{ClassName.method_name} @@ -778,7 +798,7 @@ private Object resolveExpression(String directive, String[] args, Object current // class.method_name (lowercase) if (dotIndex >= 0) { final String key = javaNameToYamlName(simpleDirective); - res.add(new SafeFetchResolver(key, context)); + res.add(new SafeFetchResolver(key)); } return res; @@ -860,12 +880,23 @@ private String javaNameToYamlName(String expression) { * {@link Name} then this method would return {@link Name#firstName()}. Returns null if the directive is nested * (i.e. has a '.') or the method doesn't exist on the obj object. */ - private ValueResolver resolveFromMethodOn(Object obj, String directive, String[] args) { + private ValueResolver resolveFromMethodOn(Object obj, String directive, String[] args, ProviderRegistration root) { if (obj == null) { return null; } final MethodAndCoercedArgs accessor = retrieveMethodAccessor(obj, directive, args); - return accessor == null ? NULL_VALUE : new MethodAndCoercedArgsResolver(accessor, obj); + if (accessor == null) return NULL_VALUE; + if (obj instanceof ProviderRegistration) { + return new RootCoercedResolver(accessor); + } + if (obj instanceof AbstractProvider) { + String providerName = obj.getClass().getSimpleName(); + Object registered = root.getProvider(providerName); + if (registered != null && registered.getClass() == obj.getClass()) { + return new NamedProviderCoercedResolver(providerName, accessor); + } + } + return new InstanceCoercedResolver(accessor, obj); } /** @@ -896,7 +927,7 @@ private ValueResolver resolveFakerObjectAndMethod(ProviderRegistration faker, St return NULL_VALUE; } - return new MethodAndCoercedArgsResolver(accessor, objectWithMethodToInvoke); + return new ChainedCoercedResolver(fakerAccessor, accessor); } catch (InvocationTargetException | IllegalAccessException e) { throw new RuntimeException("Failed to resolve faker object and method for %s (dotIndex=%s, args=%s)" .formatted(key, dotIndex, Arrays.toString(args)), e); @@ -936,8 +967,8 @@ private MethodAndCoercedArgs accessor(Class clazz, final String accessorName, }); Collection methods = classMethodsMap.getOrDefault(name, emptyList()); - if (methods.isEmpty()) { - LOG.fine(() -> "Didn't find accessor named %s on %s with args %s".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args))); + if (methods == null) { + LOG.fine(() -> "Didn't accessor named %s on %s with args %s (methods=%s)".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args), null)); return null; } LOG.fine(() -> "Found accessor named %s on %s in cache: %s".formatted(accessorName, clazz.getSimpleName(), methods)); @@ -956,7 +987,7 @@ private MethodAndCoercedArgs accessor(Class clazz, final String accessorName, if (mostRestrictive != null) { return new MethodAndCoercedArgs(mostRestrictive, coercedArgumentsForMostRestrictive); } - LOG.fine(() -> "Didn't find accessor named %s on %s with args %s (methods=%s)".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args), methods)); + LOG.fine(() -> "Didn't accessor named %s on %s with args %s (methods=%s)".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args), methods)); return null; } @@ -1148,16 +1179,19 @@ public String toString() { } } - private record RegExpContext(String exp, ProviderRegistration root, FakerContext context) { + private record CacheKey(String exp, SingletonLocale locale) { } private interface ValueResolver { - Object resolve(); + Object resolve(ProviderRegistration root, FakerContext context); + default boolean cacheable() { return true; } + /** Produce a fast per-instance resolver with provider pre-bound. Default: self (already fast or context-dependent). */ + default ValueResolver materialize(ProviderRegistration root) { return this; } } private record ConstantResolver(String value) implements ValueResolver { @Override - public Object resolve() { + public Object resolve(ProviderRegistration root, FakerContext context) { return value; } } @@ -1165,39 +1199,107 @@ public Object resolve() { private static final ConstantResolver EMPTY_STRING = new ConstantResolver(""); private static final ConstantResolver NULL_VALUE = new ConstantResolver(null); - private record MethodResolver(Method method, Object current, Object[] args) implements ValueResolver { + /** L2: fast resolver — method pre-bound to a specific provider instance. Never stored in L1. */ + private record InstanceMethodResolver(Object provider, Method method, Object[] args) implements ValueResolver { @Override - public Object resolve() { + public Object resolve(ProviderRegistration root, FakerContext context) { try { - return method.invoke(current); + return method.invoke(provider); } catch (Exception e) { throw new RuntimeException("Failed to call method %s.%s() on %s (args: %s)".formatted( - method.getDeclaringClass().getName(), method.getName(), current, Arrays.toString(args)), e); + method.getDeclaringClass().getName(), method.getName(), provider, Arrays.toString(args)), e); } } + @Override + public boolean cacheable() { return false; } + } + + /** L1 recipe: resolves a no-arg method on a named provider looked up from root at call time. */ + private record ProviderMethodResolver(String providerName, Method method, Object[] args) implements ValueResolver { + @Override + public Object resolve(ProviderRegistration root, FakerContext context) { + return new InstanceMethodResolver(root.getProvider(providerName), method, args).resolve(root, context); + } + + @Override + public ValueResolver materialize(ProviderRegistration root) { + return new InstanceMethodResolver(root.getProvider(providerName), method, args); + } + @Override public String toString() { - return "%s[method=%s.%s(), current=%s, args=%s]".formatted(getClass().getSimpleName(), - method.getDeclaringClass().getSimpleName(), method.getName(), current, Arrays.toString(args)); + return "%s[provider=%s, method=%s.%s(), args=%s]".formatted(getClass().getSimpleName(), + providerName, method.getDeclaringClass().getSimpleName(), method.getName(), Arrays.toString(args)); } } - private record MethodAndCoercedArgsResolver(MethodAndCoercedArgs accessor, Object obj) implements ValueResolver { + /** L1 recipe: resolves a coerced-args method directly on the root ProviderRegistration. */ + private record RootCoercedResolver(MethodAndCoercedArgs accessor) implements ValueResolver { @Override - public Object resolve() { - return invokeAndToString(accessor, obj); + public Object resolve(ProviderRegistration root, FakerContext context) { + return invokeCoerced(accessor, root); } - private static Object invokeAndToString(MethodAndCoercedArgs accessor, Object objectWithMethodToInvoke) { + @Override + public ValueResolver materialize(ProviderRegistration root) { + return new InstanceCoercedResolver(accessor, root); + } + } + + /** L1 recipe: resolves a coerced-args method on a named provider looked up from root at call time. */ + private record NamedProviderCoercedResolver(String providerName, MethodAndCoercedArgs accessor) implements ValueResolver { + @Override + public Object resolve(ProviderRegistration root, FakerContext context) { + return invokeCoerced(accessor, root.getProvider(providerName)); + } + + @Override + public ValueResolver materialize(ProviderRegistration root) { + return new InstanceCoercedResolver(accessor, root.getProvider(providerName)); + } + } + + /** L1 recipe: two-step chain — invokes fakerAccessor on root to get provider, then accessor on it. */ + private record ChainedCoercedResolver(MethodAndCoercedArgs fakerAccessor, MethodAndCoercedArgs accessor) implements ValueResolver { + @Override + public Object resolve(ProviderRegistration root, FakerContext context) { + try { + return invokeCoerced(accessor, fakerAccessor.invoke(root)); + } catch (InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException("Failed to invoke chained resolver on %s".formatted(root), unwrap(e)); + } + } + + @Override + public ValueResolver materialize(ProviderRegistration root) { try { - return accessor.invoke(objectWithMethodToInvoke); + return new InstanceCoercedResolver(accessor, fakerAccessor.invoke(root)); } catch (InvocationTargetException | IllegalAccessException e) { - throw new RuntimeException("Failed to invoke %s on %s".formatted(accessor, objectWithMethodToInvoke), unwrap(e)); + throw new RuntimeException("Failed to materialize chained resolver on %s".formatted(root), unwrap(e)); } } } + /** L2: fast resolver — coerced method pre-bound to a specific instance. Also used for non-registered providers (never in L1). */ + private record InstanceCoercedResolver(MethodAndCoercedArgs accessor, Object instance) implements ValueResolver { + @Override + public Object resolve(ProviderRegistration root, FakerContext context) { + return invokeCoerced(accessor, instance); + } + + @Override + public boolean cacheable() { return false; } + } + + private static Object invokeCoerced(MethodAndCoercedArgs accessor, Object target) { + try { + return accessor.invoke(target); + } catch (InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException("Failed to invoke %s on %s".formatted(accessor, target), unwrap(e)); + } + } + private static Throwable unwrap(Throwable e) { return e instanceof InvocationTargetException reflection ? unwrap(reflection.getTargetException()) : e; } diff --git a/src/test/java/net/datafaker/SharedFakeValuesServiceTest.java b/src/test/java/net/datafaker/SharedFakeValuesServiceTest.java new file mode 100644 index 000000000..0f5a139f9 --- /dev/null +++ b/src/test/java/net/datafaker/SharedFakeValuesServiceTest.java @@ -0,0 +1,86 @@ +package net.datafaker; + +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Random; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +class SharedFakeValuesServiceTest { + + @Test + void concurrentFakersProduceNoErrors() throws Exception { + int threads = 16; + int iterations = 10_000; + ExecutorService pool = Executors.newFixedThreadPool(threads); + CyclicBarrier barrier = new CyclicBarrier(threads); + List> futures = new ArrayList<>(); + for (int i = 0; i < threads; i++) { + final long seed = i; + futures.add(pool.submit(() -> { + barrier.await(); + Faker faker = new Faker(Locale.ENGLISH, new Random(seed)); + for (int j = 0; j < iterations; j++) { + assertThat(faker.name().fullName()).isNotNull(); + assertThat(faker.address().city()).isNotNull(); + assertThat(faker.internet().emailAddress()).isNotNull(); + } + return null; + })); + } + pool.shutdown(); + assertThat(pool.awaitTermination(120, TimeUnit.SECONDS)).isTrue(); + for (Future f : futures) { + f.get(); + } + } + + @Test + void multipleLocalesConcurrentlyProduceNoErrors() throws Exception { + Locale[] locales = {Locale.ENGLISH, Locale.FRENCH, Locale.GERMAN, Locale.forLanguageTag("es")}; + int threads = locales.length * 4; + ExecutorService pool = Executors.newFixedThreadPool(threads); + CyclicBarrier barrier = new CyclicBarrier(threads); + List> futures = new ArrayList<>(); + for (int i = 0; i < threads; i++) { + final Locale locale = locales[i % locales.length]; + futures.add(pool.submit(() -> { + barrier.await(); + Faker faker = new Faker(locale, new Random()); + for (int j = 0; j < 1_000; j++) { + assertThat(faker.name().fullName()).isNotNull(); + assertThat(faker.address().city()).isNotNull(); + } + return null; + })); + } + pool.shutdown(); + assertThat(pool.awaitTermination(60, TimeUnit.SECONDS)).isTrue(); + for (Future f : futures) { + f.get(); + } + } + + @Test + void deterministicOutputUnaffectedByCaching() { + long seed = 12345L; + Faker first = new Faker(Locale.ENGLISH, new Random(seed)); + String firstName = first.name().firstName(); + String city = first.address().city(); + String email = first.internet().emailAddress(); + + // A second Faker with the same seed must produce the same output regardless of cache state + Faker second = new Faker(Locale.ENGLISH, new Random(seed)); + assertThat(second.name().firstName()).isEqualTo(firstName); + assertThat(second.address().city()).isEqualTo(city); + assertThat(second.internet().emailAddress()).isEqualTo(email); + } +} From d41e2309e2594777a63e6ab35ab5085f43a01e71 Mon Sep 17 00:00:00 2001 From: Marco Ferretti Date: Thu, 4 Jun 2026 17:23:37 +0200 Subject: [PATCH 2/5] Renamed private method resExp to resolveExpression for clarity. --- .../java/net/datafaker/service/FakeValuesService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/datafaker/service/FakeValuesService.java b/src/main/java/net/datafaker/service/FakeValuesService.java index 3926fd07c..eaa975776 100644 --- a/src/main/java/net/datafaker/service/FakeValuesService.java +++ b/src/main/java/net/datafaker/service/FakeValuesService.java @@ -618,8 +618,8 @@ protected String resolveExpression(String expression, Object current, ProviderRe while (j < length && Character.isWhitespace(expr.charAt(j))) j++; final String arguments = j == length ? "" : expr.substring(j); final String[] args = splitArguments(arguments); - resolved = resExp(directive, args, current, root, context, cacheKey); - // resExp stored recipe in RECIPE_MAP if cacheable; materialize for L2 + resolved = resolveExpression(directive, args, current, root, context, cacheKey); + // resolveExpression stored recipe in RECIPE_MAP if cacheable; materialize for L2 final ValueResolver stored = RECIPE_MAP.get(cacheKey); if (stored != null) { instanceMap.put(expr, stored.materialize(root)); @@ -699,9 +699,9 @@ private String[] splitExpressions(String expression, int length) { }); } - private Object resExp(String directive, String[] args, Object current, ProviderRegistration root, FakerContext context, CacheKey cacheKey) { + private Object resolveExpression(String directive, String[] args, Object current, ProviderRegistration root, FakerContext context, CacheKey cacheKey) { Object res = resolveExpression(directive, args, current, root, context); - LOG.fine(() -> "resExp(%s [%s]) current: %s, root: %s, context: %s, cacheKey: %s -> res: %s".formatted(directive, Arrays.toString(args), current, root, context, cacheKey, res)); + LOG.fine(() -> "resolveExpression(%s [%s]) current: %s, root: %s, context: %s, cacheKey: %s -> res: %s".formatted(directive, Arrays.toString(args), current, root, context, cacheKey, res)); if (res instanceof CharSequence) { if (((CharSequence) res).isEmpty()) { RECIPE_MAP.put(cacheKey, EMPTY_STRING); From 4d2bb1fc79f83c9b2fe36828abf262b886456ef8 Mon Sep 17 00:00:00 2001 From: Marco Ferretti Date: Thu, 4 Jun 2026 17:28:53 +0200 Subject: [PATCH 3/5] Address Copilot review: null guards, fix dead code, add RECIPE_MAP growth note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SafeFetchResolver.resolve(): guard against null root - resolveFromMethodOn(): guard null root before NamedProviderCoercedResolver branch - resolveExpression inner: guard null root before dotIndex provider lookup - RECIPE_MAP: expand Javadoc noting bounded-in-practice growth - accessor(): fix dead-code `methods == null` check → `methods.isEmpty()` - accessor(): fix two "Didn't accessor" log typos → "Didn't find accessor" - SharedFakeValuesServiceTest: call shutdownNow() on awaitTermination timeout Co-Authored-By: Claude Sonnet 4.6 --- .../datafaker/service/FakeValuesService.java | 18 ++++++++++++------ .../datafaker/SharedFakeValuesServiceTest.java | 10 ++++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/datafaker/service/FakeValuesService.java b/src/main/java/net/datafaker/service/FakeValuesService.java index eaa975776..06c80d2e1 100644 --- a/src/main/java/net/datafaker/service/FakeValuesService.java +++ b/src/main/java/net/datafaker/service/FakeValuesService.java @@ -81,7 +81,12 @@ public class FakeValuesService { private static final Map EXPRESSION_2_SPLITTED = new CopyOnWriteMap<>(WeakHashMap::new); - /** L1: static recipe cache — context-free resolvers shared across all Fakers with same locale. */ + /** + * L1: static recipe cache — context-free resolvers shared across all Fakers with same locale. + * Growth is bounded in practice by unique YAML expressions × locales. User-supplied dynamic + * expressions via {@code faker.expression()} carry the same theoretical unbounded-growth exposure + * as the other static string caches in this class (NAME_2_YAML, EXPRESSION_2_SPLITTED, etc.). + */ private static final Map RECIPE_MAP = new CopyOnWriteMap<>(HashMap::new); /** L2: per-instance materialized cache — resolvers pre-bound to this Faker's providers for fast repeated calls. */ @@ -179,6 +184,7 @@ private SafeFetchResolver(String simpleDirective) { @Override public Object resolve(ProviderRegistration root, FakerContext context) { + if (root == null) return null; return root.fakeValuesService().safeFetch(simpleDirective, context, null); } @@ -759,7 +765,7 @@ private Object resolveExpression(String directive, String[] args, Object current } res.add(resolveFromMethodOn(current, directive, args, root)); } - if (dotIndex > 0) { + if (dotIndex > 0 && root != null) { String providerClassName = directive.substring(0, dotIndex); String methodName = directive.substring(dotIndex + 1); AbstractProvider ap = root.getProvider(providerClassName); @@ -889,7 +895,7 @@ private ValueResolver resolveFromMethodOn(Object obj, String directive, String[] if (obj instanceof ProviderRegistration) { return new RootCoercedResolver(accessor); } - if (obj instanceof AbstractProvider) { + if (obj instanceof AbstractProvider && root != null) { String providerName = obj.getClass().getSimpleName(); Object registered = root.getProvider(providerName); if (registered != null && registered.getClass() == obj.getClass()) { @@ -967,8 +973,8 @@ private MethodAndCoercedArgs accessor(Class clazz, final String accessorName, }); Collection methods = classMethodsMap.getOrDefault(name, emptyList()); - if (methods == null) { - LOG.fine(() -> "Didn't accessor named %s on %s with args %s (methods=%s)".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args), null)); + if (methods.isEmpty()) { + LOG.fine(() -> "Didn't find accessor named %s on %s with args %s".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args))); return null; } LOG.fine(() -> "Found accessor named %s on %s in cache: %s".formatted(accessorName, clazz.getSimpleName(), methods)); @@ -987,7 +993,7 @@ private MethodAndCoercedArgs accessor(Class clazz, final String accessorName, if (mostRestrictive != null) { return new MethodAndCoercedArgs(mostRestrictive, coercedArgumentsForMostRestrictive); } - LOG.fine(() -> "Didn't accessor named %s on %s with args %s (methods=%s)".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args), methods)); + LOG.fine(() -> "Didn't find accessor named %s on %s with args %s (methods=%s)".formatted(accessorName, clazz.getSimpleName(), Arrays.toString(args), methods)); return null; } diff --git a/src/test/java/net/datafaker/SharedFakeValuesServiceTest.java b/src/test/java/net/datafaker/SharedFakeValuesServiceTest.java index 0f5a139f9..301a1fb76 100644 --- a/src/test/java/net/datafaker/SharedFakeValuesServiceTest.java +++ b/src/test/java/net/datafaker/SharedFakeValuesServiceTest.java @@ -37,7 +37,10 @@ void concurrentFakersProduceNoErrors() throws Exception { })); } pool.shutdown(); - assertThat(pool.awaitTermination(120, TimeUnit.SECONDS)).isTrue(); + if (!pool.awaitTermination(120, TimeUnit.SECONDS)) { + pool.shutdownNow(); + throw new AssertionError("Thread pool did not terminate within timeout"); + } for (Future f : futures) { f.get(); } @@ -63,7 +66,10 @@ void multipleLocalesConcurrentlyProduceNoErrors() throws Exception { })); } pool.shutdown(); - assertThat(pool.awaitTermination(60, TimeUnit.SECONDS)).isTrue(); + if (!pool.awaitTermination(60, TimeUnit.SECONDS)) { + pool.shutdownNow(); + throw new AssertionError("Thread pool did not terminate within timeout"); + } for (Future f : futures) { f.get(); } From 0d52a2b06d267739f0b74a6c913d8c29ac38c620 Mon Sep 17 00:00:00 2001 From: Marco Ferretti Date: Thu, 4 Jun 2026 17:34:09 +0200 Subject: [PATCH 4/5] Guard null root in ProviderMethodResolver and outer materialize calls - ProviderMethodResolver.resolve(): return null when root is null - ProviderMethodResolver.materialize(): return this when root is null - resolveExpression outer loop: skip L2 caching when root is null, call recipe directly without materializing Co-Authored-By: Claude Sonnet 4.6 --- .../java/net/datafaker/service/FakeValuesService.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/datafaker/service/FakeValuesService.java b/src/main/java/net/datafaker/service/FakeValuesService.java index 06c80d2e1..0c7d1981c 100644 --- a/src/main/java/net/datafaker/service/FakeValuesService.java +++ b/src/main/java/net/datafaker/service/FakeValuesService.java @@ -612,8 +612,8 @@ protected String resolveExpression(String expression, Object current, ProviderRe // L1: static recipe hit — materialize once, store in L2 final ValueResolver recipe = RECIPE_MAP.get(cacheKey); if (recipe != null) { - final ValueResolver materialized = recipe.materialize(root); - instanceMap.put(expr, materialized); + final ValueResolver materialized = root != null ? recipe.materialize(root) : recipe; + if (root != null) instanceMap.put(expr, materialized); resolved = materialized.resolve(root, context); } else { // Both miss: full discovery @@ -627,7 +627,7 @@ protected String resolveExpression(String expression, Object current, ProviderRe resolved = resolveExpression(directive, args, current, root, context, cacheKey); // resolveExpression stored recipe in RECIPE_MAP if cacheable; materialize for L2 final ValueResolver stored = RECIPE_MAP.get(cacheKey); - if (stored != null) { + if (stored != null && root != null) { instanceMap.put(expr, stored.materialize(root)); } } @@ -1225,11 +1225,13 @@ public Object resolve(ProviderRegistration root, FakerContext context) { private record ProviderMethodResolver(String providerName, Method method, Object[] args) implements ValueResolver { @Override public Object resolve(ProviderRegistration root, FakerContext context) { + if (root == null) return null; return new InstanceMethodResolver(root.getProvider(providerName), method, args).resolve(root, context); } @Override public ValueResolver materialize(ProviderRegistration root) { + if (root == null) return this; return new InstanceMethodResolver(root.getProvider(providerName), method, args); } From 93bdd347d1f2d92b8db98746c2d33378c0c48fcf Mon Sep 17 00:00:00 2001 From: Marco Ferretti Date: Thu, 4 Jun 2026 17:48:44 +0200 Subject: [PATCH 5/5] Add null-root guards to remaining L1 resolvers RootCoercedResolver, NamedProviderCoercedResolver, and ChainedCoercedResolver all called root.getProvider() / fakerAccessor.invoke(root) without guarding against null root in both resolve() and materialize(). Pattern matches the existing guards on SafeFetchResolver and ProviderMethodResolver. Co-Authored-By: Claude Sonnet 4.6 --- src/main/java/net/datafaker/service/FakeValuesService.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/net/datafaker/service/FakeValuesService.java b/src/main/java/net/datafaker/service/FakeValuesService.java index 0c7d1981c..f675ab03f 100644 --- a/src/main/java/net/datafaker/service/FakeValuesService.java +++ b/src/main/java/net/datafaker/service/FakeValuesService.java @@ -1246,11 +1246,13 @@ public String toString() { private record RootCoercedResolver(MethodAndCoercedArgs accessor) implements ValueResolver { @Override public Object resolve(ProviderRegistration root, FakerContext context) { + if (root == null) return null; return invokeCoerced(accessor, root); } @Override public ValueResolver materialize(ProviderRegistration root) { + if (root == null) return this; return new InstanceCoercedResolver(accessor, root); } } @@ -1259,11 +1261,13 @@ public ValueResolver materialize(ProviderRegistration root) { private record NamedProviderCoercedResolver(String providerName, MethodAndCoercedArgs accessor) implements ValueResolver { @Override public Object resolve(ProviderRegistration root, FakerContext context) { + if (root == null) return null; return invokeCoerced(accessor, root.getProvider(providerName)); } @Override public ValueResolver materialize(ProviderRegistration root) { + if (root == null) return this; return new InstanceCoercedResolver(accessor, root.getProvider(providerName)); } } @@ -1272,6 +1276,7 @@ public ValueResolver materialize(ProviderRegistration root) { private record ChainedCoercedResolver(MethodAndCoercedArgs fakerAccessor, MethodAndCoercedArgs accessor) implements ValueResolver { @Override public Object resolve(ProviderRegistration root, FakerContext context) { + if (root == null) return null; try { return invokeCoerced(accessor, fakerAccessor.invoke(root)); } catch (InvocationTargetException | IllegalAccessException e) { @@ -1281,6 +1286,7 @@ public Object resolve(ProviderRegistration root, FakerContext context) { @Override public ValueResolver materialize(ProviderRegistration root) { + if (root == null) return this; try { return new InstanceCoercedResolver(accessor, fakerAccessor.invoke(root)); } catch (InvocationTargetException | IllegalAccessException e) {