Merge pull request #3 from Greybird/JENKINS-48725

[JENKINS-48725] fix listener exceptions not being handled properly
jenkinsci · Jan 23, 2018 · 32147b8 · 32147b8
2 parents 8d7e39f + b35e46b
commit 32147b8
Show file tree

Hide file tree

Showing 4 changed files with 231 additions and 46 deletions.
diff --git a/pom.xml b/pom.xml
@@ -14,6 +14,13 @@
     <java.level>8</java.level>
   </properties>
 
+  <repositories>
+    <repository>
+      <id>repo.jenkins-ci.org</id>
+      <url>http://repo.jenkins-ci.org/public/</url>
+    </repository>
+  </repositories>
+
   <dependencies>
     <dependency>
       <groupId>com.google.code.findbugs</groupId>

diff --git a/src/main/java/org/jvnet/hudson/reactor/Reactor.java b/src/main/java/org/jvnet/hudson/reactor/Reactor.java
@@ -173,7 +173,11 @@ private synchronized Node milestone(final Milestone m) {
         if (n==null) {
             milestones.put(m,n=new Node(new Runnable() {
                 public void run() {
-                    listener.onAttained(m);
+                    try {
+                        listener.onAttained(m);
+                    } catch(Throwable x) {
+                        throw new TunnelException(x);
+                    }
                 }
                 public String toString() {
                     return "Milestone:"+m.toString();
@@ -205,15 +209,24 @@ public synchronized void addAll(Iterable<? extends Task> _tasks) {
         for (final Task t : _tasks) {
             Node n = new Node(new Runnable() {
                 public void run() {
-                    listener.onTaskStarted(t);
                     try {
+                        listener.onTaskStarted(t);
                         runTask(t);
                         listener.onTaskCompleted(t);
                     } catch (Throwable x) {
                         boolean fatal = t.failureIsFatal();
-                        listener.onTaskFailed(t,x, fatal);
+                        TunnelException te = null;
+                        try {
+                            listener.onTaskFailed(t, x, fatal);
+                        } catch(Throwable x2) {
+                            te = new TunnelException(x2);
+                            x2.addSuppressed(x);
+                        }
+                        if (te == null) {
+                            te = new TunnelException(x);
+                        }
                         if (fatal)
-                            throw new TunnelException(x);
+                            throw te;
                     }
                 }
 
@@ -265,8 +278,9 @@ public synchronized void execute(final Executor e, final ReactorListener listene
             // block until everything is done
             while(pending>0) {
                 wait();
-                if (fatal!=null)
+                if (fatal!=null) {
                     throw new ReactorException(fatal.getCause());
+                }
             }
         } finally {
             // avoid memory leak

diff --git a/src/main/java/org/jvnet/hudson/reactor/ReactorListener.java b/src/main/java/org/jvnet/hudson/reactor/ReactorListener.java
@@ -23,6 +23,7 @@
  */
 package org.jvnet.hudson.reactor;
 
+import java.util.concurrent.Callable;
 import java.util.concurrent.Executor;
 import java.util.Collection;
 
@@ -86,23 +87,45 @@ public Aggregator(Collection<ReactorListener> listeners) {
         }
 
         public void onTaskStarted(Task t) {
-            for (ReactorListener listener : listeners)
-                listener.onTaskStarted(t);
+            run(l -> l.onTaskStarted(t));
         }
 
         public void onTaskCompleted(Task t) {
-            for (ReactorListener listener : listeners)
-                listener.onTaskCompleted(t);
+            run(l -> l.onTaskCompleted(t));
         }
 
         public void onTaskFailed(Task t, Throwable err, boolean fatal) {
-            for (ReactorListener listener : listeners)
-                listener.onTaskFailed(t,err,fatal);
+            run(l -> l.onTaskFailed(t,err,fatal));
         }
 
         public void onAttained(Milestone milestone) {
-            for (ReactorListener listener : listeners)
-                listener.onAttained(milestone);
+            run(l -> l.onAttained(milestone));
+        }
+
+        private void run(ListenerAction action) {
+            Throwable ex = null;
+            for (ReactorListener listener : listeners) {
+                try {
+                    action.run(listener);
+                } catch (Throwable x) {
+                    if (ex == null) {
+                        ex = x;
+                    } else {
+                        ex.addSuppressed(x);
+                    }
+                }
+            }
+            if (ex != null) {
+                if (ex instanceof Error) {
+                    throw (Error) ex;
+                } else if (ex instanceof RuntimeException) {
+                    throw (RuntimeException) ex;
+                }
+            }
+        }
+
+        private interface ListenerAction {
+            void run(ReactorListener listener);
         }
     }
 }