Make arquillian-core thread-safe for parallel JUnit 5/6 class execution

Signed-off-by: Olivier Lamy <olamy@apache.org>

Author: olamy

container/impl-base/src/main/java/org/jboss/arquillian/container/impl/ContainerImpl.java

@@ -60,8 +60,8 @@ public class ContainerImpl<T extends ContainerConfiguration> implements Containe
 
     private DeployableContainer<T> deployableContainer;
     private String name;
-    private State state = State.STOPPED;
-    private Throwable failureCause;
+    private volatile State state = State.STOPPED;
+    private volatile Throwable failureCause;
 
     private ContainerDef containerConfiguration;
 

core/impl-base/src/main/java/org/jboss/arquillian/core/impl/ManagerImpl.java

@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
 import org.jboss.arquillian.core.api.Injector;
 import org.jboss.arquillian.core.api.annotation.ApplicationScoped;
 import org.jboss.arquillian.core.api.event.ManagerStarted;
@@ -75,8 +76,8 @@ protected Set<Class<? extends Throwable>> initialValue() {
         };
 
     ManagerImpl(final Collection<Class<? extends Context>> contextClasses, final Collection<Class<?>> extensionClasses) {
-        this.contexts = new ArrayList<Context>();
-        this.extensions = new ArrayList<Extension>();
+        this.contexts = new CopyOnWriteArrayList<Context>();
+        this.extensions = new CopyOnWriteArrayList<Extension>();
         this.runtimeLogger = new RuntimeLogger();
 
         try {

core/spi/src/main/java/org/jboss/arquillian/core/spi/ArquillianThreadLocal.java

@@ -1,13 +1,22 @@
 package org.jboss.arquillian.core.spi;
 
-import java.util.Hashtable;
-
 /**
- * Mapping for ThreadId to a value. Same as "ThreadLocal", but with simpler cleanup.
- *
+ * Thread-local value holder backed by a real {@link ThreadLocal}.
+ * A prior implementation used a map keyed by thread id, which leaked stale
+ * values when a pool recycled a thread's id.
  */
 public class ArquillianThreadLocal<T> {
-  private Hashtable<Long, T> table = new Hashtable<Long, T>();
+
+  private volatile ThreadLocal<T> delegate = newDelegate();
+
+  private ThreadLocal<T> newDelegate() {
+    return new ThreadLocal<T>() {
+      @Override
+      protected T initialValue() {
+        return ArquillianThreadLocal.this.initialValue();
+      }
+    };
+  }
 
   protected T initialValue() {
     return null;
@@ -22,38 +31,21 @@ protected T initialValue() {
    * @return the current thread's value of this thread-local
    */
   public T get() {
-    Thread t = Thread.currentThread();
-    long threadId = t.getId();
-
-    if (table.containsKey(threadId)) {
-        return table.get(threadId);
-    }
-    else {
-      T value = initialValue();
-      table.put(threadId, value);
-      return value;
-    }
+    return delegate.get();
   }
 
   /**
-   * Removes the current thread's value for this thread-local
-   * variable.
-   *
+   * Removes the current thread's value for this thread-local variable.
    */
-   public void remove() {
-     Thread t = Thread.currentThread();
-     long threadId = t.getId();
-
-     if (table.containsKey(threadId)) {
-       table.remove(threadId);
-     }
-   }
+  public void remove() {
+    delegate.remove();
+  }
 
-   /**
-    * Clears the cache
-    */
-   public void clear() {
-     table.clear();
-   }
+  /**
+   * Drops every thread's value by swapping in a fresh backing {@link ThreadLocal};
+   * subsequent {@link #get()} calls go back through {@link #initialValue()}.
+   */
+  public void clear() {
+    delegate = newDelegate();
+  }
 }
-

junit5/core/src/main/java/org/jboss/arquillian/junit5/JUnitJupiterTestClassLifecycleManager.java

@@ -4,23 +4,18 @@
 import org.jboss.arquillian.test.spi.TestRunnerAdaptorBuilder;
 import org.junit.jupiter.api.extension.ExtensionContext;
 
-import static org.jboss.arquillian.junit5.ContextStore.getContextStore;
-
 /**
- * Manages the lifecycle of JUnit Jupiter test classes within the Arquillian framework.
- * This class implements both {@link AutoCloseable} and {@link ExtensionContext.Store.CloseableResource}
- * for junit5 prior to 5.13.0 to ensure proper resource cleanup.
+ * Owns the {@link TestRunnerAdaptor} for a single JUnit Jupiter test class.
  *
- * <p>The manager is responsible for:</p>
- * <ul>
- *   <li>Initializing and managing the Arquillian{@link TestRunnerAdaptor}</li>
- *   <li>Handling test suite lifecycle events (beforeSuite, afterSuite)</li>
- *   <li>Providing access to the test runner adaptor for test execution</li>
- *   <li>Managing initialization failures and error handling</li>
- * </ul>
+ * <p>The manager is cached in the root store under a {@link ExtensionContext.Namespace}
+ * keyed by the test class, so each class gets its own {@code TestRunnerAdaptor}
+ * and underlying {@code Manager}. This matters under parallel class execution:
+ * a shared {@code Manager} would leak thread-local context activations between
+ * classes running on different threads.</p>
  *
- * <p>This class uses a singleton pattern per test context, ensuring that only one
- * manager instance exists per test suite execution.</p>
+ * <p>Implementing {@link ExtensionContext.Store.CloseableResource} lets JUnit
+ * call {@link #close()} (and therefore {@code afterSuite}) when it tears down
+ * the root context at the end of the run.</p>
  */
 
 public class JUnitJupiterTestClassLifecycleManager implements AutoCloseable,
@@ -35,14 +30,23 @@ private JUnitJupiterTestClassLifecycleManager() {
     }
 
     static JUnitJupiterTestClassLifecycleManager getManager(ExtensionContext context) throws Exception {
-        ExtensionContext.Store store = getContextStore(context).getRootStore();
-        JUnitJupiterTestClassLifecycleManager instance = store.get(MANAGER_KEY, JUnitJupiterTestClassLifecycleManager.class);
-        if (instance == null) {
-            instance = new JUnitJupiterTestClassLifecycleManager();
-            store.put(MANAGER_KEY, instance);
-            instance.initializeAdaptor();
-        }
-        // no, initialization has been attempted before and failed, refuse
+        ExtensionContext.Store store = context.getRoot().getStore(
+            ExtensionContext.Namespace.create(
+                JUnitJupiterTestClassLifecycleManager.class,
+                context.getRequiredTestClass()));
+        JUnitJupiterTestClassLifecycleManager instance = store.getOrComputeIfAbsent(
+            MANAGER_KEY,
+            key -> {
+                JUnitJupiterTestClassLifecycleManager mgr = new JUnitJupiterTestClassLifecycleManager();
+                try {
+                    mgr.initializeAdaptor();
+                } catch (Exception e) {
+                    mgr.caughtInitializationException = e;
+                }
+                return mgr;
+            },
+            JUnitJupiterTestClassLifecycleManager.class);
+        // initialization has been attempted before and failed, refuse
         // to do anything else
         if (instance.hasInitializationException()) {
             instance.handleSuiteLevelFailure();