From 7e3242b5ccbb0ae06e65929bce7dec4218db5509 Mon Sep 17 00:00:00 2001
From: Luke Butters <lbutters@funnelback.com>
Date: Tue, 29 May 2018 13:43:17 +1000
Subject: [PATCH] Stop creating unnecessary exceptions with MultiException
 #2580

This doesn't actually stop creating the unnecessary exception instead it
stop running the expensive `fillInStackTrace()` unless we actually need
it.

This does result in the stack trace being set to where we throw the
exception rather than when we create it.

Signed-off-by: Luke Butters <lbutters@funnelback.com>
---
 .../eclipse/jetty/util/MultiException.java    | 22 ++++++++++++++
 .../jetty/util/MultiExceptionTest.java        | 30 ++++++++++++++-----
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java
index c51b0e66acd..f45b4cdaa7a 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java
@@ -31,11 +31,28 @@ import java.util.List;
 public class MultiException extends Exception
 {
     private List<Throwable> nested;
+    boolean superConstructorCalled = false;
 
     /* ------------------------------------------------------------ */
     public MultiException()
     {
         super("Multiple exceptions");
+        this.superConstructorCalled = true;
+    }
+    
+    /**
+     * We only want to fill in the stack trace if we have an exception to throw.
+     * 
+     * This prevents the stack trace from being written when the constructor is
+     * called, yet will let as fill it in later on e.g. when we actually want
+     * to throw the multi exception.
+     */
+    @Override
+    public Throwable fillInStackTrace() {
+        if(superConstructorCalled) {
+            return super.fillInStackTrace();
+        }
+        return this;
     }
 
     /* ------------------------------------------------------------ */
@@ -105,6 +122,7 @@ public class MultiException extends Exception
               if (th instanceof Exception)
                   throw (Exception)th;
           default:
+              this.fillInStackTrace();
               throw this;
         }
     }
@@ -137,6 +155,7 @@ public class MultiException extends Exception
               else
                   throw new RuntimeException(th);
           default:
+              this.fillInStackTrace();
               throw new RuntimeException(this);
         }
     }
@@ -155,7 +174,10 @@ public class MultiException extends Exception
             return;
         
         if (nested.size()>0)
+        {
+            this.fillInStackTrace();
             throw this;
+        }
     }
 
     /* ------------------------------------------------------------ */
diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java
index 091d4373fcc..44d7e1af5b3 100644
--- a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java
+++ b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java
@@ -21,10 +21,10 @@ package org.eclipse.jetty.util;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
-import java.io.IOException;
-
 import org.junit.Test;
 
+import java.io.IOException;
+
 
 public class MultiExceptionTest
 {
@@ -37,6 +37,8 @@ public class MultiExceptionTest
         me.ifExceptionThrow();
         me.ifExceptionThrowMulti();
         me.ifExceptionThrowRuntime();
+        
+        assertEquals("Stack trace should not be filled out", 0, me.getStackTrace().length);
     }
 
     @Test
@@ -91,19 +93,26 @@ public class MultiExceptionTest
         {
             assertTrue(run==e);
         }
+        
+        assertEquals("Stack trace should not be filled out", 0, me.getStackTrace().length);
     }
 
-    @Test
-    public void testTwo() throws Exception
-    {
+    private MultiException multiExceptionWithTwo() {
         MultiException me = new MultiException();
         IOException io = new IOException("one");
         RuntimeException run = new RuntimeException("one");
         me.add(io);
         me.add(run);
-
         assertEquals(2,me.size());
-
+        
+        assertEquals("Stack trace should not be filled out", 0, me.getStackTrace().length);
+        return me;
+    }
+    
+    @Test
+    public void testTwo() throws Exception
+    {
+        MultiException me = multiExceptionWithTwo();
         try
         {
             me.ifExceptionThrow();
@@ -112,8 +121,10 @@ public class MultiExceptionTest
         catch(MultiException e)
         {
             assertTrue(e==me);
+            assertTrue(e.getStackTrace().length > 0);
         }
 
+        me = multiExceptionWithTwo();
         try
         {
             me.ifExceptionThrowMulti();
@@ -122,8 +133,10 @@ public class MultiExceptionTest
         catch(MultiException e)
         {
             assertTrue(e==me);
+            assertTrue(e.getStackTrace().length > 0);
         }
 
+        me = multiExceptionWithTwo();
         try
         {
             me.ifExceptionThrowRuntime();
@@ -132,8 +145,10 @@ public class MultiExceptionTest
         catch(RuntimeException e)
         {
             assertTrue(e.getCause()==me);
+            assertTrue(e.getStackTrace().length > 0);
         }
 
+        RuntimeException run = new RuntimeException("one");
         me = new MultiException();
         me.add(run);
         me.add(run);
@@ -146,6 +161,7 @@ public class MultiExceptionTest
         catch(RuntimeException e)
         {
             assertTrue(e.getCause()==me);
+            assertTrue(e.getStackTrace().length > 0);
         }
     }