Fixed random "recursive update during ServiceLoading" exception

AB-xdev · AB-xdev · commit 2dfa5780e7c8 · 2025-07-31T14:28:13.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+# 2.4.1
+* Fixed random "recursive update during ServiceLoading" exception #342
+
 # 2.4.0
 * ``oidc-server-mock``
   * Improved extensibility by creating abstract base classes
diff --git a/base-demo/src/test/java/software/xdev/tci/factory/prestart/PreStartDemoTest.java b/base-demo/src/test/java/software/xdev/tci/factory/prestart/PreStartDemoTest.java
@@ -9,7 +9,7 @@
 import software.xdev.tci.dummyinfra.factory.DummyTCIFactory;
 import software.xdev.tci.factory.prestart.config.PreStartConfig;
 import software.xdev.tci.factory.registry.TCIFactoryRegistry;
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 
 
 class PreStartDemoTest
@@ -23,7 +23,8 @@ class PreStartDemoTest
 	static void beforeAll()
 	{
 		// Force enable PreStarting
-		TCIServiceLoader.instance().forceOverwrite(PreStartConfig.class, new PreStartConfig()
+		TCIServiceLoaderHolder.instance().forceOverwrite(
+			PreStartConfig.class, new PreStartConfig()
 		{
 			@Override
 			public boolean enabled()
diff --git a/base/src/main/java/software/xdev/tci/envperf/EnvironmentPerformance.java b/base/src/main/java/software/xdev/tci/envperf/EnvironmentPerformance.java
@@ -16,7 +16,7 @@
 package software.xdev.tci.envperf;
 
 import software.xdev.tci.envperf.impl.TCIEnvironmentPerformance;
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 
 
 /**
@@ -34,7 +34,7 @@ public static int cpuSlownessFactor()
 	
 	public static TCIEnvironmentPerformance impl()
 	{
-		return TCIServiceLoader.instance().service(TCIEnvironmentPerformance.class);
+		return TCIServiceLoaderHolder.instance().service(TCIEnvironmentPerformance.class);
 	}
 	
 	private EnvironmentPerformance()
diff --git a/base/src/main/java/software/xdev/tci/factory/prestart/config/PreStartConfig.java b/base/src/main/java/software/xdev/tci/factory/prestart/config/PreStartConfig.java
@@ -15,7 +15,7 @@
  */
 package software.xdev.tci.factory.prestart.config;
 
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 
 
 public interface PreStartConfig
@@ -91,6 +91,6 @@ default boolean detectEndingTests()
 	
 	static PreStartConfig instance()
 	{
-		return TCIServiceLoader.instance().service(PreStartConfig.class);
+		return TCIServiceLoaderHolder.instance().service(PreStartConfig.class);
 	}
 }
diff --git a/base/src/main/java/software/xdev/tci/factory/prestart/coordinator/GlobalPreStartCoordinator.java b/base/src/main/java/software/xdev/tci/factory/prestart/coordinator/GlobalPreStartCoordinator.java
@@ -16,7 +16,7 @@
 package software.xdev.tci.factory.prestart.coordinator;
 
 import software.xdev.tci.factory.prestart.PreStartableTCIFactory;
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 
 
 public interface GlobalPreStartCoordinator extends AutoCloseable
@@ -30,11 +30,11 @@ public interface GlobalPreStartCoordinator extends AutoCloseable
 	
 	static GlobalPreStartCoordinator instance()
 	{
-		return TCIServiceLoader.instance().service(GlobalPreStartCoordinator.class);
+		return TCIServiceLoaderHolder.instance().service(GlobalPreStartCoordinator.class);
 	}
 	
 	static boolean isPresent()
 	{
-		return TCIServiceLoader.instance().isLoaded(GlobalPreStartCoordinator.class);
+		return TCIServiceLoaderHolder.instance().isLoaded(GlobalPreStartCoordinator.class);
 	}
 }
diff --git a/base/src/main/java/software/xdev/tci/factory/prestart/loadbalancing/LoadMonitor.java b/base/src/main/java/software/xdev/tci/factory/prestart/loadbalancing/LoadMonitor.java
@@ -17,7 +17,7 @@
 
 import java.util.OptionalDouble;
 
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 
 
 /**
@@ -35,6 +35,6 @@ public interface LoadMonitor
 	
 	static LoadMonitor instance()
 	{
-		return TCIServiceLoader.instance().service(LoadMonitor.class);
+		return TCIServiceLoaderHolder.instance().service(LoadMonitor.class);
 	}
 }
diff --git a/base/src/main/java/software/xdev/tci/factory/registry/TCIFactoryRegistry.java b/base/src/main/java/software/xdev/tci/factory/registry/TCIFactoryRegistry.java
@@ -21,7 +21,7 @@
 
 import software.xdev.tci.TCI;
 import software.xdev.tci.factory.TCIFactory;
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 
 
 /**
@@ -45,6 +45,6 @@ public interface TCIFactoryRegistry
 	
 	static TCIFactoryRegistry instance()
 	{
-		return TCIServiceLoader.instance().service(TCIFactoryRegistry.class);
+		return TCIServiceLoaderHolder.instance().service(TCIFactoryRegistry.class);
 	}
 }
diff --git a/base/src/main/java/software/xdev/tci/leakdetection/TCILeakAgent.java b/base/src/main/java/software/xdev/tci/leakdetection/TCILeakAgent.java
@@ -37,7 +37,7 @@
 import software.xdev.tci.factory.TCIFactory;
 import software.xdev.tci.factory.registry.TCIFactoryRegistry;
 import software.xdev.tci.leakdetection.config.LeakDetectionConfig;
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 
 
 /**
@@ -58,7 +58,7 @@ public class TCILeakAgent implements TestExecutionListener
 	@Override
 	public void testPlanExecutionStarted(final TestPlan testPlan)
 	{
-		this.config = TCIServiceLoader.instance().service(LeakDetectionConfig.class);
+		this.config = TCIServiceLoaderHolder.instance().service(LeakDetectionConfig.class);
 		if(!this.config.enabled())
 		{
 			return;
diff --git a/base/src/main/java/software/xdev/tci/serviceloading/TCIProviderPriority.java b/base/src/main/java/software/xdev/tci/serviceloading/TCIProviderPriority.java
@@ -22,7 +22,7 @@
 
 
 /**
- * Priority of a provider for {@link TCIServiceLoader}.
+ * Priority of a provider for {@link TCIServiceLoaderHolder}.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.TYPE)
diff --git a/base/src/main/java/software/xdev/tci/serviceloading/TCIServiceLoader.java b/base/src/main/java/software/xdev/tci/serviceloading/TCIServiceLoader.java
@@ -16,50 +16,115 @@
 package software.xdev.tci.serviceloading;
 
 import java.util.Comparator;
+import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Optional;
 import java.util.ServiceLoader;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
 
 
 /**
  * Central point for service loading
  */
-@SuppressWarnings("java:S6548")
-public final class TCIServiceLoader
+@SuppressWarnings({"java:S6548", "java:S2789"}) // Don't force us to write our own Optional!?!
+public class TCIServiceLoader
 {
-	private static final TCIServiceLoader INSTANCE = new TCIServiceLoader();
+	protected final Object globalInitServiceLockHandlerLock = new Object();
+	protected final InheritableThreadLocal<LinkedHashSet<Class<?>>> tlDetectRecursiveInitServices =
+		new InheritableThreadLocal<>();
+	protected final Map<Class<?>, ReentrantLock> servicesLoadingSyncLocks = new ConcurrentHashMap<>();
+	protected final Map<Class<?>, Optional<?>> loadedServices = new ConcurrentHashMap<>();
 	
-	public static TCIServiceLoader instance()
+	public <T> T service(final Class<T> clazz)
 	{
-		return INSTANCE;
+		// Do not use computeIfAbsent here. This might cause "recursive updates" exceptions.
+		// These are usually not recursive, there is just a call here doing another call with a different type
+		// Recursive updates will be detected by initService
+		
+		if(!this.loadedServices.containsKey(clazz))
+		{
+			this.initService(clazz);
+		}
+		
+		return this.loadedServices.get(clazz)
+			.map(clazz::cast)
+			.orElse(null);
 	}
 	
-	private final Map<Class<?>, Object> loadedServices = new ConcurrentHashMap<>();
-	
-	private TCIServiceLoader()
+	/**
+	 * @implNote This method uses best effort thread safety. It WILL fail when another Thread that didn't inherit from
+	 * the current Thread is causing a deadlock, for example <code>ForkJoinPool.commonPool</code>
+	 */
+	protected <T> void initService(final Class<T> clazz)
 	{
+		final LinkedHashSet<Class<?>> detectRecursiveInitServices;
+		final ReentrantLock lock;
+		synchronized(this.globalInitServiceLockHandlerLock)
+		{
+			detectRecursiveInitServices = Optional.ofNullable(this.tlDetectRecursiveInitServices.get())
+				.map(LinkedHashSet::new)
+				.orElseGet(LinkedHashSet::new);
+			
+			if(detectRecursiveInitServices.contains(clazz))
+			{
+				throw new IllegalStateException("Detected recursive initialization on "
+					+ clazz
+					+ "; chain trying class creation: "
+					+ detectRecursiveInitServices.stream()
+					.map(Class::getName)
+					.collect(Collectors.joining(" -> ")));
+			}
+			
+			detectRecursiveInitServices.add(clazz);
+			this.tlDetectRecursiveInitServices.set(detectRecursiveInitServices);
+			
+			lock = this.servicesLoadingSyncLocks.computeIfAbsent(
+				clazz,
+				ignored -> new ReentrantLock());
+			lock.lock();
+		}
+		
+		try
+		{
+			// Already initialized?
+			if(this.loadedServices.containsKey(clazz))
+			{
+				return;
+			}
+			
+			this.loadedServices.put(clazz, this.loadService(clazz));
+		}
+		finally
+		{
+			synchronized(this.globalInitServiceLockHandlerLock)
+			{
+				detectRecursiveInitServices.remove(clazz);
+				this.tlDetectRecursiveInitServices.set(detectRecursiveInitServices);
+				
+				this.servicesLoadingSyncLocks.remove(clazz);
+				lock.unlock();
+			}
+		}
 	}
 	
-	@SuppressWarnings("unchecked")
-	public <T> T service(final Class<T> clazz)
+	protected <T> Optional<T> loadService(final Class<T> clazz)
 	{
-		return (T)this.loadedServices.computeIfAbsent(
-			clazz,
-			c -> ServiceLoader.load(clazz)
-				.stream()
-				// Get by highest priority
-				.max(Comparator.comparingInt(p ->
-					Optional.ofNullable(p.type().getAnnotation(TCIProviderPriority.class))
-						.map(TCIProviderPriority::value)
-						.orElse(TCIProviderPriority.DEFAULT_PRIORITY)))
-				.map(ServiceLoader.Provider::get)
-				.orElse(null));
+		return ServiceLoader.load(clazz)
+			.stream()
+			// Get by highest priority
+			.max(Comparator.comparingInt(p ->
+				Optional.ofNullable(p.type().getAnnotation(TCIProviderPriority.class))
+					.map(TCIProviderPriority::value)
+					.orElse(TCIProviderPriority.DEFAULT_PRIORITY)))
+			.map(ServiceLoader.Provider::get);
 	}
 	
 	public boolean isLoaded(final Class<?> clazz)
 	{
-		return this.loadedServices.get(clazz) != null;
+		final Optional<?> optImpl = this.loadedServices.get(clazz);
+		return optImpl != null && optImpl.isPresent();
 	}
 	
 	/**
@@ -68,8 +133,8 @@ public boolean isLoaded(final Class<?> clazz)
 	 * WARNING: Usage not recommended. Should only be used as last resort!
 	 * </p>
 	 */
-	public Object forceOverwrite(final Class<?> clazz, final Object value)
+	public void forceOverwrite(final Class<?> clazz, final Object value)
 	{
-		return this.loadedServices.put(clazz, value);
+		this.loadedServices.put(clazz, Optional.ofNullable(value));
 	}
 }
diff --git a/base/src/main/java/software/xdev/tci/serviceloading/TCIServiceLoaderHolder.java b/base/src/main/java/software/xdev/tci/serviceloading/TCIServiceLoaderHolder.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright © 2024 XDEV Software (https://xdev.software)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package software.xdev.tci.serviceloading;
+
+public final class TCIServiceLoaderHolder
+{
+	static final TCIServiceLoader INSTANCE = new TCIServiceLoader();
+	
+	public static TCIServiceLoader instance()
+	{
+		return INSTANCE;
+	}
+	
+	private TCIServiceLoaderHolder()
+	{
+	}
+}
diff --git a/base/src/main/java/software/xdev/tci/tracing/TCITracingAgent.java b/base/src/main/java/software/xdev/tci/tracing/TCITracingAgent.java
@@ -31,7 +31,7 @@
 
 import software.xdev.tci.factory.TCIFactory;
 import software.xdev.tci.factory.registry.TCIFactoryRegistry;
-import software.xdev.tci.serviceloading.TCIServiceLoader;
+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;
 import software.xdev.tci.tracing.config.TracingConfig;
 
 
@@ -60,7 +60,7 @@ public class TCITracingAgent implements TestExecutionListener
 	@Override
 	public void testPlanExecutionStarted(final TestPlan testPlan)
 	{
-		this.config = TCIServiceLoader.instance().service(TracingConfig.class);
+		this.config = TCIServiceLoaderHolder.instance().service(TracingConfig.class);
 		if(!this.config.enabled())
 		{
 			return;
diff --git a/base/src/test/java/software/xdev/tci/serviceloading/TCIServiceLoaderTest.java b/base/src/test/java/software/xdev/tci/serviceloading/TCIServiceLoaderTest.java
diff --git a/selenium/src/main/java/software/xdev/tci/selenium/factory/BrowserTCIFactory.java b/selenium/src/main/java/software/xdev/tci/selenium/factory/BrowserTCIFactory.java

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@`
`16`	`16`	`package software.xdev.tci.envperf;`
`17`	`17`
`18`	`18`	`import software.xdev.tci.envperf.impl.TCIEnvironmentPerformance;`
`19`		`-import software.xdev.tci.serviceloading.TCIServiceLoader;`
	`19`	`+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;`
`20`	`20`
`21`	`21`
`22`	`22`	`/**`
`@@ -34,7 +34,7 @@ public static int cpuSlownessFactor()`
`34`	`34`
`35`	`35`	`public static TCIEnvironmentPerformance impl()`
`36`	`36`	`{`
`37`		`- return TCIServiceLoader.instance().service(TCIEnvironmentPerformance.class);`
	`37`	`+ return TCIServiceLoaderHolder.instance().service(TCIEnvironmentPerformance.class);`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`private EnvironmentPerformance()`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@`
`15`	`15`	`*/`
`16`	`16`	`package software.xdev.tci.factory.prestart.config;`
`17`	`17`
`18`		`-import software.xdev.tci.serviceloading.TCIServiceLoader;`
	`18`	`+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;`
`19`	`19`
`20`	`20`
`21`	`21`	`public interface PreStartConfig`
`@@ -91,6 +91,6 @@ default boolean detectEndingTests()`
`91`	`91`
`92`	`92`	`static PreStartConfig instance()`
`93`	`93`	`{`
`94`		`- return TCIServiceLoader.instance().service(PreStartConfig.class);`
	`94`	`+ return TCIServiceLoaderHolder.instance().service(PreStartConfig.class);`
`95`	`95`	`}`
`96`	`96`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@`
`17`	`17`
`18`	`18`	`import java.util.OptionalDouble;`
`19`	`19`
`20`		`-import software.xdev.tci.serviceloading.TCIServiceLoader;`
	`20`	`+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;`
`21`	`21`
`22`	`22`
`23`	`23`	`/**`
`@@ -35,6 +35,6 @@ public interface LoadMonitor`
`35`	`35`
`36`	`36`	`static LoadMonitor instance()`
`37`	`37`	`{`
`38`		`- return TCIServiceLoader.instance().service(LoadMonitor.class);`
	`38`	`+ return TCIServiceLoaderHolder.instance().service(LoadMonitor.class);`
`39`	`39`	`}`
`40`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@`
`21`	`21`
`22`	`22`	`import software.xdev.tci.TCI;`
`23`	`23`	`import software.xdev.tci.factory.TCIFactory;`
`24`		`-import software.xdev.tci.serviceloading.TCIServiceLoader;`
	`24`	`+import software.xdev.tci.serviceloading.TCIServiceLoaderHolder;`
`25`	`25`
`26`	`26`
`27`	`27`	`/**`
`@@ -45,6 +45,6 @@ public interface TCIFactoryRegistry`
`45`	`45`
`46`	`46`	`static TCIFactoryRegistry instance()`
`47`	`47`	`{`
`48`		`- return TCIServiceLoader.instance().service(TCIFactoryRegistry.class);`
	`48`	`+ return TCIServiceLoaderHolder.instance().service(TCIFactoryRegistry.class);`
`49`	`49`	`}`
`50`	`50`	`}`