using COWAL + backward compatible + SynchronizedRunListener

Author: Tibor Digana

Diff for: NOTICE.txt

@@ -2,4 +2,6 @@
    ==  Notices and attributions required by libraries that the project depends on   ==
    ===================================================================================
 
- The JUnit depends on Java Hamcrest (http://hamcrest.org/JavaHamcrest/).
+ The JUnit depends on Java Hamcrest (http://hamcrest.org/JavaHamcrest).
+
+ The JUnit depends on net.jcip:jcip-annotations:1.0 (http://jcip.net).

Diff for: build.xml

@@ -30,7 +30,10 @@
   <property name="javadoczip" location="${dist}-javadoc.zip" />
   <property name="hamcrestlib" location="lib/hamcrest-core-1.3.jar" />
   <property name="hamcrestlibsources" location="lib/hamcrest-core-1.3-sources.jar" />
+  <property name="jciplib" location="lib/jcip-annotations-1.0.jar" />
+  <property name="jciplibsources" location="lib/jcip-annotations-1.0-sources.jar" />
   <property name="hamcrestsrc" location="${dist}/temp.hamcrest.source" />
+  <property name="jcipsrc" location="${dist}/temp.jcip.source" />
 
   <property name="maven.deploy.goal" value="org.apache.maven.plugins:maven-gpg-plugin:1.1:sign-and-deploy-file" />
 
@@ -80,9 +83,10 @@
   </macrodef>
 
   <target name="build" depends="versiontag">
-    <junit_compilation srcdir="${src}" destdir="${bin}" classpath="${hamcrestlib}"/>
+    <junit_compilation srcdir="${src}" destdir="${bin}" classpath="${hamcrestlib};${jciplib}"/>
     <unjar src="${hamcrestlib}" dest="${bin}" />
-    <junit_compilation srcdir="${testsrc}" destdir="${testbin}" classpath="${hamcrestlib};${bin}"/>
+    <unjar src="${jciplib}" dest="${bin}" />
+    <junit_compilation srcdir="${testsrc}" destdir="${testbin}" classpath="${hamcrestlib};${jciplib};${bin}"/>
   </target>
 
   <target name="jars" depends="build">
@@ -128,6 +132,10 @@
   <target name="unjar.hamcrest">
     <unjar src="${hamcrestlibsources}" dest="${hamcrestsrc}" />
   </target>
+
+  <target name="unjar.jcip">
+    <unjar src="${jciplibsources}" dest="${jcipsrc}" />
+  </target>
 
   <target name="release-notes">
     <property name="basename" value="doc/ReleaseNotes${version-base}" />
@@ -138,20 +146,21 @@
     </exec>
   </target>
 
-  <target name="javadoc" depends="unjar.hamcrest">
+  <target name="javadoc" depends="unjar.hamcrest, unjar.jcip">
     <javadoc destdir="${javadocdir}"
              author="false"
              version="false"
              use="false"
              windowtitle="JUnit API"
-             stylesheetfile="stylesheet.css"
+             stylesheetfile="src/main/javadoc/stylesheet.css"
              >
       <excludepackage name="junit.*" />
       <excludepackage name="org.junit.internal.*" />
       <excludepackage name="org.junit.experimental.theories.internal.*" />
 
       <sourcepath location="${src}" />
       <sourcepath location="${hamcrestsrc}" />
+      <sourcepath location="${jcipsrc}" />
       <link href="http://java.sun.com/javase/6/docs/api/" />
     </javadoc>
   </target>
@@ -198,6 +207,7 @@
           <pathelement location="${bin}" />
           <pathelement location="${testbin}" />
           <pathelement location="${hamcrestlib}" />
+          <pathelement location="${jciplib}" />
         </classpath>
       </java>    
     </sequential>

Diff for: lib/jcip-annotations-1.0-sources.jar

@@ -261,6 +261,7 @@
                     <includeDependencySources>true</includeDependencySources>
                     <dependencySourceIncludes>
                         <dependencySourceInclude>org.hamcrest:hamcrest-core:*</dependencySourceInclude>
+                        <dependencySourceInclude>net.jcip:jcip-annotations:*</dependencySourceInclude>
                     </dependencySourceIncludes>
                 </configuration>
             </plugin>

Diff for: lib/jcip-annotations-1.0.jar

@@ -1,5 +1,6 @@
 package org.junit.runner;
 
+import net.jcip.annotations.ThreadSafe;
 import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunListener;
 
@@ -65,6 +66,7 @@ public boolean wasSuccessful() {
         return getFailureCount() == 0;
     }
 
+    @ThreadSafe
     private class Listener extends RunListener {
         @Override
         public void testRunStarted(Description description) throws Exception {

Diff for: pom.xml

@@ -8,7 +8,8 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * If you write custom runners, you may need to notify JUnit of your progress running tests.
@@ -20,92 +21,35 @@
  * @since 4.0
  */
 public class RunNotifier {
-    private final ReentrantLock lock = new ReentrantLock();
-
-    private volatile RunListener[] listeners = new RunListener[0];
+    private final CopyOnWriteArrayList<RunListener> listeners = new CopyOnWriteArrayList<RunListener>();
     private volatile boolean pleaseStop = false;
 
     private static RunListener wrapSynchronizedIfNotThreadSafe(RunListener listener) {
         boolean isThreadSafe = listener.getClass().isAnnotationPresent(ThreadSafe.class);
         return isThreadSafe ? listener : new SynchronizedRunListener(listener);
     }
 
-    /**
-     * Satisfies <tt>(o == null ? e == null : o.equals(e)</tt>
-     * in {@link java.util.List#remove(Object)}.
-     *
-     * @param o listener to remove
-     * @param e element in <code>listeners</code> which was previously added
-     * @return {@code true} if <code>o</code> is equal with <code>e</code>
-     */
-    private static boolean equalListeners(Object o, Object e) {
-        if (o == null) {
-            return e == null;
-        } else {
-            return e.getClass() == SynchronizedRunListener.class ? e.equals(o) : o.equals(e);
-        }
-    }
-
     /**
      * Internal use only
      */
     public void addListener(RunListener listener) {
-        if (listener != null) {
-            listener = wrapSynchronizedIfNotThreadSafe(listener);
-            final ReentrantLock lock = this.lock;
-            lock.lock();
-            try {
-                // same behavior as List#add(Object)
-                RunListener[] elements = this.listeners;
-                int length = elements.length;
-                RunListener[] listeners = new RunListener[1 + length];
-                for (int i = 0; i < length; ++i) {
-                    listeners[i] = elements[i];
-                }
-                listeners[length] = listener;
-                this.listeners = listeners;
-            } finally {
-                lock.unlock();
-            }
-        }
+        listener = wrapSynchronizedIfNotThreadSafe(listener);
+        listeners.add(listener);
     }
 
     /**
      * Internal use only
      */
     public void removeListener(RunListener listener) {
-        if (listener != null) {
-            final ReentrantLock lock = this.lock;
-            lock.lock();
-            try {
-                // same behavior as List#remove(Object)
-                RunListener[] elements = this.listeners;
-                int length = elements.length;
-                if (length > 0) {
-                    RunListener[] listeners = new RunListener[length - 1];
-                    for (int i = 0, newLength = listeners.length; i < length; ++i) {
-                        if (equalListeners(listener, elements[i])) {
-                            for (int k = 1 + i; k != Integer.MAX_VALUE && k < length; ++k) {
-                                listeners[k - 1] = elements[k];
-                            }
-                            this.listeners = listeners;
-                            return;
-                        } else if (i < newLength) {
-                            listeners[i] = elements[i];
-                        }
-                    }
-                }
-            } finally {
-                lock.unlock();
-            }
-        }
+        listener = wrapSynchronizedIfNotThreadSafe(listener);
+        listeners.remove(listener);
     }
 
     private abstract class SafeNotifier {
         private final Collection<RunListener> currentListeners;
 
         SafeNotifier() {
-            this(Arrays.asList(listeners));
+            this(listeners);
         }
 
         SafeNotifier(Collection<RunListener> currentListeners) {
@@ -178,10 +122,10 @@ protected void notifyListener(RunListener each) throws Exception {
      * @param failure the description of the test that failed and the exception thrown
      */
     public void fireTestFailure(Failure failure) {
-        fireTestFailures(Arrays.asList(listeners), Arrays.asList(failure));
+        fireTestFailures(listeners, Arrays.asList(failure));
     }
 
-    private void fireTestFailures(Collection<RunListener> listeners, final Collection<Failure> failures) {
+    private void fireTestFailures(Collection<RunListener> listeners, final List<Failure> failures) {
         if (!failures.isEmpty()) {
             new SafeNotifier(listeners) {
                 @Override
@@ -254,22 +198,6 @@ public void pleaseStop() {
      * Internal use only. The Result's listener must be first.
      */
     public void addFirstListener(RunListener listener) {
-        if (listener != null) {
-            listener = wrapSynchronizedIfNotThreadSafe(listener);
-            final ReentrantLock lock = this.lock;
-            lock.lock();
-            try {
-                // same behavior as List#add(0, Object)
-                RunListener[] elements = this.listeners;
-                RunListener[] listeners = new RunListener[1 + elements.length];
-                listeners[0] = listener;
-                for (int i = 0, length = elements.length; i < length; ++i) {
-                    listeners[1 + i] = elements[i];
-                }
-                this.listeners = listeners;
-            } finally {
-                lock.unlock();
-            }
-        }
+        listeners.add(0, listener);
     }
 }

Diff for: src/main/java/org/junit/runner/Result.java

@@ -1,5 +1,6 @@
 package org.junit.runner.notification;
 
+import net.jcip.annotations.ThreadSafe;
 import org.junit.runner.Description;
 import org.junit.runner.Result;
 
@@ -11,6 +12,7 @@
  * @version 4.12
  * @since 4.12
  */
+@ThreadSafe
 final class SynchronizedRunListener extends RunListener {
     private final RunListener listener;
 
@@ -60,7 +62,16 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object o) {
-        return o.equals(listener);
+        if (o == this) {
+            return true;
+        }
+
+        if (o instanceof SynchronizedRunListener) {
+            SynchronizedRunListener other = (SynchronizedRunListener) o;
+            return listener.equals(other.listener);
+        } else {
+            return listener.equals(o);
+        }
     }
 
     @Override

Diff for: src/main/java/org/junit/runner/notification/RunNotifier.java

@@ -5,8 +5,9 @@
 import org.junit.internal.MethodSorterTest;
 import org.junit.internal.matchers.StacktracePrintingMatcherTest;
 import org.junit.runner.RunWith;
-import org.junit.runner.notification.RunNotifierTest;
+import org.junit.runner.notification.AddRemoveListenerTest;
 import org.junit.runner.notification.ConcurrentRunNotifierTest;
+import org.junit.runner.notification.RunNotifierTest;
 import org.junit.runners.Suite;
 import org.junit.runners.Suite.SuiteClasses;
 import org.junit.tests.assertion.AssertionTest;
@@ -170,7 +171,8 @@
         StacktracePrintingMatcherTest.class,
         StopwatchTest.class,
         RunNotifierTest.class,
-        ConcurrentRunNotifierTest.class
+        ConcurrentRunNotifierTest.class,
+        AddRemoveListenerTest.class
 })
 public class AllTests {
     public static Test suite() {