Add synchronization to iterations of ServiceLoader collections

Author: sbabcoc

src/main/java/com/nordstrom/automation/junit/LifecycleHooks.java

@@ -151,22 +151,28 @@ public static void intercept(@This final Object runner, @SuperCall final Callabl
                 CHILD_TO_PARENT.put(child, runner);
             }
 
-            if (NOTIFIERS.add(notifier)) {
-                Description description = invoke(runner, "getDescription");
-                for (RunListener listener : runListenerLoader) {
-                    notifier.addListener(listener);
-                    listener.testRunStarted(description);
+            synchronized(NOTIFIERS) {
+                if (NOTIFIERS.add(notifier)) {
+                    Description description = invoke(runner, "getDescription");
+                    for (RunListener listener : runListenerLoader) {
+                        notifier.addListener(listener);
+                        listener.testRunStarted(description);
+                    }
                 }
             }
 
-            for (RunnerWatcher watcher : runnerWatcherLoader) {
-                watcher.runStarted(runner);
+            synchronized(runnerWatcherLoader) {
+                for (RunnerWatcher watcher : runnerWatcherLoader) {
+                    watcher.runStarted(runner);
+                }
             }
 
             callProxy(proxy);
 
-            for (RunnerWatcher watcher : runnerWatcherLoader) {
-                watcher.runFinished(runner);
+            synchronized(runnerWatcherLoader) {
+                for (RunnerWatcher watcher : runnerWatcherLoader) {
+                    watcher.runFinished(runner);
+                }
             }
         }
 
@@ -210,8 +216,10 @@ public static Object intercept(@This final Object runner,
             TARGET_TO_TESTCLASS.put(testObj, getTestClassOf(runner));
             applyTimeout(testObj);
 
-            for (TestObjectWatcher watcher : objectWatcherLoader) {
-                watcher.testObjectCreated(testObj, TARGET_TO_TESTCLASS.get(testObj));
+            synchronized(objectWatcherLoader) {
+                for (TestObjectWatcher watcher : objectWatcherLoader) {
+                    watcher.testObjectCreated(testObj, TARGET_TO_TESTCLASS.get(testObj));
+                }
             }
 
             return testObj;

src/main/java/com/nordstrom/automation/junit/RetryHandler.java

@@ -129,9 +129,11 @@ static int getMaxRetry(Object runner, final FrameworkMethod method) {
      * @return {@code true} if test should be retried; otherwise {@code false}
      */
     static boolean isRetriable(final FrameworkMethod method, final Throwable thrown) {
-        for (JUnitRetryAnalyzer analyzer : retryAnalyzerLoader) {
-            if (analyzer.retry(method, thrown)) {
-                return true;
+        synchronized(retryAnalyzerLoader) {
+            for (JUnitRetryAnalyzer analyzer : retryAnalyzerLoader) {
+                if (analyzer.retry(method, thrown)) {
+                    return true;
+                }
             }
         }
         return false;

src/main/java/com/nordstrom/automation/junit/RunReflectiveCall.java

@@ -77,17 +77,21 @@ public static Object intercept(@This final Object callable, @SuperCall final Cal
 
         Object result = null;
         Throwable thrown = null;
-        for (MethodWatcher watcher : methodWatcherLoader) {
-            watcher.beforeInvocation(target, method, params);
+        synchronized(methodWatcherLoader) {
+            for (MethodWatcher watcher : methodWatcherLoader) {
+                watcher.beforeInvocation(target, method, params);
+            }
         }
 
         try {
             result = LifecycleHooks.callProxy(proxy);
         } catch (Throwable t) {
             thrown = t;
         } finally {
-            for (MethodWatcher watcher : methodWatcherLoader) {
-                watcher.afterInvocation(target, method, thrown);
+            synchronized(methodWatcherLoader) {
+                for (MethodWatcher watcher : methodWatcherLoader) {
+                    watcher.afterInvocation(target, method, thrown);
+                }
             }
         }
 
@@ -108,8 +112,10 @@ public static Object intercept(@This final Object callable, @SuperCall final Cal
     static void fireTestStarted(TestClass testClass, Runnable runnable) {
         AtomicTest atomicTest = createAtomicTest(testClass, runnable);
         if (atomicTest != null) {
-            for (RunWatcher watcher : runWatcherLoader) {
-                watcher.testStarted(atomicTest.getIdentity(), atomicTest.getTestClass());
+            synchronized(runWatcherLoader) {
+                for (RunWatcher watcher : runWatcherLoader) {
+                    watcher.testStarted(atomicTest.getIdentity(), atomicTest.getTestClass());
+                }
             }
         }
     }
@@ -122,9 +128,11 @@ static void fireTestStarted(TestClass testClass, Runnable runnable) {
     static void fireTestFinished(TestClass testClass) {
         AtomicTest atomicTest = TESTCLASS_TO_ATOMICTEST.get(testClass);
         if (atomicTest != null) {
-            for (RunWatcher watcher : runWatcherLoader) {
-                notifyIfTestFailed(watcher, atomicTest);
-                watcher.testFinished(atomicTest.getIdentity(), atomicTest.getTestClass());
+            synchronized(runWatcherLoader) {
+                for (RunWatcher watcher : runWatcherLoader) {
+                    notifyIfTestFailed(watcher, atomicTest);
+                    watcher.testFinished(atomicTest.getIdentity(), atomicTest.getTestClass());
+                }
             }
         }
     }
@@ -155,8 +163,10 @@ private static void notifyIfTestFailed(RunWatcher watcher, AtomicTest atomicTest
      */
     static void fireTestIgnored(Object runner, FrameworkMethod method) {
         TestClass testClass = getTestClassOf(runner);
-        for (RunWatcher watcher : runWatcherLoader) {
-            watcher.testIgnored(method, testClass);
+        synchronized(runWatcherLoader) {
+            for (RunWatcher watcher : runWatcherLoader) {
+                watcher.testIgnored(method, testClass);
+            }
         }
     }
 
@@ -183,9 +193,11 @@ public static Object getTargetFor(FrameworkMethod method) {
     public static Optional<MethodWatcher> getAttachedWatcher(
                     Class<? extends MethodWatcher> watcherType) {
         Objects.requireNonNull(watcherType, "[watcherType] must be non-null");
-        for (MethodWatcher watcher : methodWatcherLoader) {
-            if (watcher.getClass() == watcherType) {
-                return Optional.of(watcher);
+        synchronized(methodWatcherLoader) {
+            for (MethodWatcher watcher : methodWatcherLoader) {
+                if (watcher.getClass() == watcherType) {
+                    return Optional.of(watcher);
+                }
             }
         }
         return Optional.empty();