From 9b012d25b8555a5eddb6fd47aafe6b931abe8fe6 Mon Sep 17 00:00:00 2001
From: "Neil A. Wilson" <neilwilson@pingidentity.com>
Date: Tue, 28 Aug 2018 12:20:27 -0500
Subject: [PATCH] Better handling for Throwable

Improved the LDAP SDK's behavior in some cases where it caught
Throwable and could suppress an Error that would have been better to
re-throw back to the caller, after ensuring that any appropriate
cleanup is performed.  In some cases, the same is also true for
RuntimeException.
---
 docs/release-notes.html                       |   8 ++
 .../LDAPListenerClientConnection.java         |   2 +-
 .../ldap/sdk/AbstractConnectionPool.java      |   2 +
 src/com/unboundid/ldap/sdk/ConnectThread.java |   1 +
 .../ldap/sdk/LDAPConnectionPool.java          |   2 +
 .../sdk/LDAPThreadLocalConnectionPool.java    |   2 +
 .../sdk/persist/DefaultObjectEncoder.java     |  10 +-
 .../ldap/sdk/persist/LDAPObjectHandler.java   |  35 ++---
 src/com/unboundid/util/StaticUtils.java       |  95 +++++++++++++
 .../AsynchronousParallelProcessor.java        |   3 +-
 .../util/parallel/ParallelProcessor.java      |   2 +-
 .../unboundid/util/StaticUtilsTestCase.java   | 134 ++++++++++++++++++
 12 files changed, 271 insertions(+), 25 deletions(-)

diff --git a/docs/release-notes.html b/docs/release-notes.html
index f0309627f..1c5514a19 100644
--- a/docs/release-notes.html
+++ b/docs/release-notes.html
@@ -13,6 +13,14 @@ <h3>Version 4.0.8</h3>
               </p>
 
               <ul>
+                <li>
+                  Updated a number of locations in the code that caught <tt>Throwable</tt> to
+                  re-throw the original <tt>Throwable</tt> instance (after performing appropriate
+                  cleanup) if that <tt>Throwable</tt> instance was an <tt>Error</tt> or perhaps a
+                  <tt>RuntimeException</tt>.
+                  <br><br>
+                </li>
+
                 <li>
                   Added a number of convenience methods to the <tt>JSONObject</tt> class that make
                   it easier to get the value of a specified field as a string, Boolean, number,
diff --git a/src/com/unboundid/ldap/listener/LDAPListenerClientConnection.java b/src/com/unboundid/ldap/listener/LDAPListenerClientConnection.java
index d1f9b645a..11824213d 100644
--- a/src/com/unboundid/ldap/listener/LDAPListenerClientConnection.java
+++ b/src/com/unboundid/ldap/listener/LDAPListenerClientConnection.java
@@ -627,7 +627,7 @@ public void run()
                ERR_CONN_EXCEPTION_IN_REQUEST_HANDLER.get(
                     String.valueOf(requestMessage),
                     StaticUtils.getExceptionMessage(t))));
-          return;
+          StaticUtils.throwErrorOrRuntimeException(t);
         }
       }
     }
diff --git a/src/com/unboundid/ldap/sdk/AbstractConnectionPool.java b/src/com/unboundid/ldap/sdk/AbstractConnectionPool.java
index cf6205122..c2a15f1dc 100644
--- a/src/com/unboundid/ldap/sdk/AbstractConnectionPool.java
+++ b/src/com/unboundid/ldap/sdk/AbstractConnectionPool.java
@@ -2996,6 +2996,7 @@ void throwLDAPException(final Throwable t, final LDAPConnection conn)
     else
     {
       releaseDefunctConnection(conn);
+      StaticUtils.rethrowIfError(t);
       throw new LDAPException(ResultCode.LOCAL_ERROR,
            ERR_POOL_OP_EXCEPTION.get(StaticUtils.getExceptionMessage(t)), t);
     }
@@ -3036,6 +3037,7 @@ void throwLDAPSearchException(final Throwable t, final LDAPConnection conn)
     else
     {
       releaseDefunctConnection(conn);
+      StaticUtils.rethrowIfError(t);
       throw new LDAPSearchException(ResultCode.LOCAL_ERROR,
            ERR_POOL_OP_EXCEPTION.get(StaticUtils.getExceptionMessage(t)), t);
     }
diff --git a/src/com/unboundid/ldap/sdk/ConnectThread.java b/src/com/unboundid/ldap/sdk/ConnectThread.java
index 110152a8d..9aec95f91 100644
--- a/src/com/unboundid/ldap/sdk/ConnectThread.java
+++ b/src/com/unboundid/ldap/sdk/ConnectThread.java
@@ -248,6 +248,7 @@ Socket getConnectedSocket()
     }
     else
     {
+      StaticUtils.rethrowIfError(cause);
       throw new LDAPException(ResultCode.CONNECT_ERROR,
            ERR_CONNECT_THREAD_EXCEPTION.get(address, port,
                 StaticUtils.getExceptionMessage(cause)), cause);
diff --git a/src/com/unboundid/ldap/sdk/LDAPConnectionPool.java b/src/com/unboundid/ldap/sdk/LDAPConnectionPool.java
index ba21a0840..087205146 100644
--- a/src/com/unboundid/ldap/sdk/LDAPConnectionPool.java
+++ b/src/com/unboundid/ldap/sdk/LDAPConnectionPool.java
@@ -1669,6 +1669,7 @@ public BindResult bindAndRevertAuthentication(final BindRequest bindRequest)
       else
       {
         releaseDefunctConnection(conn);
+        StaticUtils.rethrowIfError(t);
         throw new LDAPException(ResultCode.LOCAL_ERROR,
              ERR_POOL_OP_EXCEPTION.get(StaticUtils.getExceptionMessage(t)), t);
       }
@@ -1709,6 +1710,7 @@ public BindResult bindAndRevertAuthentication(final BindRequest bindRequest)
       else
       {
         releaseDefunctConnection(conn);
+        StaticUtils.rethrowIfError(t);
         throw new LDAPException(ResultCode.LOCAL_ERROR,
              ERR_POOL_OP_EXCEPTION.get(StaticUtils.getExceptionMessage(t)), t);
       }
diff --git a/src/com/unboundid/ldap/sdk/LDAPThreadLocalConnectionPool.java b/src/com/unboundid/ldap/sdk/LDAPThreadLocalConnectionPool.java
index c8eb824ce..ef9d59f25 100644
--- a/src/com/unboundid/ldap/sdk/LDAPThreadLocalConnectionPool.java
+++ b/src/com/unboundid/ldap/sdk/LDAPThreadLocalConnectionPool.java
@@ -767,6 +767,7 @@ public BindResult bindAndRevertAuthentication(final BindRequest bindRequest)
       else
       {
         releaseDefunctConnection(conn);
+        StaticUtils.rethrowIfError(t);
         throw new LDAPException(ResultCode.LOCAL_ERROR,
              ERR_POOL_OP_EXCEPTION.get(StaticUtils.getExceptionMessage(t)), t);
       }
@@ -807,6 +808,7 @@ public BindResult bindAndRevertAuthentication(final BindRequest bindRequest)
       else
       {
         releaseDefunctConnection(conn);
+        StaticUtils.rethrowIfError(t);
         throw new LDAPException(ResultCode.LOCAL_ERROR,
              ERR_POOL_OP_EXCEPTION.get(StaticUtils.getExceptionMessage(t)), t);
       }
diff --git a/src/com/unboundid/ldap/sdk/persist/DefaultObjectEncoder.java b/src/com/unboundid/ldap/sdk/persist/DefaultObjectEncoder.java
index cbcab2407..e44024b8b 100644
--- a/src/com/unboundid/ldap/sdk/persist/DefaultObjectEncoder.java
+++ b/src/com/unboundid/ldap/sdk/persist/DefaultObjectEncoder.java
@@ -1248,20 +1248,20 @@ else if (typeInfo.isSet() && isSupportedSetType(baseClass))
       Debug.debugException(lpe);
       throw lpe;
     }
-    catch (final Throwable t)
+    catch (final Exception e)
     {
-      Debug.debugException(t);
+      Debug.debugException(e);
 
-      if (t instanceof InvocationTargetException)
+      if (e instanceof InvocationTargetException)
       {
         final Throwable targetException =
-             ((InvocationTargetException) t).getTargetException();
+             ((InvocationTargetException) e).getTargetException();
         throw new LDAPPersistException(
              StaticUtils.getExceptionMessage(targetException), targetException);
       }
       else
       {
-        throw new LDAPPersistException(StaticUtils.getExceptionMessage(t), t);
+        throw new LDAPPersistException(StaticUtils.getExceptionMessage(e), e);
       }
     }
   }
diff --git a/src/com/unboundid/ldap/sdk/persist/LDAPObjectHandler.java b/src/com/unboundid/ldap/sdk/persist/LDAPObjectHandler.java
index eadbf31ec..6c30e9426 100644
--- a/src/com/unboundid/ldap/sdk/persist/LDAPObjectHandler.java
+++ b/src/com/unboundid/ldap/sdk/persist/LDAPObjectHandler.java
@@ -1230,14 +1230,14 @@ T decode(final Entry e)
     {
       o = constructor.newInstance();
     }
-    catch (final Throwable t)
+    catch (final Exception ex)
     {
-      Debug.debugException(t);
+      Debug.debugException(ex);
 
-      if (t instanceof InvocationTargetException)
+      if (ex instanceof InvocationTargetException)
       {
         final Throwable targetException =
-             ((InvocationTargetException) t).getTargetException();
+             ((InvocationTargetException) ex).getTargetException();
         throw new LDAPPersistException(
              ERR_OBJECT_HANDLER_ERROR_INVOKING_CONSTRUCTOR.get(type.getName(),
                   StaticUtils.getExceptionMessage(targetException)),
@@ -1247,8 +1247,8 @@ T decode(final Entry e)
       {
         throw new LDAPPersistException(
              ERR_OBJECT_HANDLER_ERROR_INVOKING_CONSTRUCTOR.get(type.getName(),
-                  StaticUtils.getExceptionMessage(t)),
-             t);
+                  StaticUtils.getExceptionMessage(ex)),
+             ex);
       }
     }
 
@@ -1299,24 +1299,25 @@ void decode(final T o, final Entry e)
       {
         postDecodeMethod.invoke(o);
       }
-      catch (final Throwable t)
+      catch (final Exception ex)
       {
-        Debug.debugException(t);
+        Debug.debugException(ex);
+        StaticUtils.rethrowIfError(ex);
 
-        if (t instanceof InvocationTargetException)
+        if (ex instanceof InvocationTargetException)
         {
-          cause = ((InvocationTargetException) t).getTargetException();
+          cause = ((InvocationTargetException) ex).getTargetException();
         }
         else
         {
-          cause = t;
+          cause = ex;
         }
 
         successful = false;
         failureReasons.add(
              ERR_OBJECT_HANDLER_ERROR_INVOKING_POST_DECODE_METHOD.get(
                   postDecodeMethod.getName(), type.getName(),
-                  StaticUtils.getExceptionMessage(t)));
+                  StaticUtils.getExceptionMessage(ex)));
       }
     }
 
@@ -1398,14 +1399,14 @@ Entry encode(final T o, final String parentDN)
       {
         postEncodeMethod.invoke(o, entry);
       }
-      catch (final Throwable t)
+      catch (final Exception ex)
       {
-        Debug.debugException(t);
+        Debug.debugException(ex);
 
-        if (t instanceof InvocationTargetException)
+        if (ex instanceof InvocationTargetException)
         {
           final Throwable targetException =
-               ((InvocationTargetException) t).getTargetException();
+               ((InvocationTargetException) ex).getTargetException();
           throw new LDAPPersistException(
                ERR_OBJECT_HANDLER_ERROR_INVOKING_POST_ENCODE_METHOD.get(
                     postEncodeMethod.getName(), type.getName(),
@@ -1417,7 +1418,7 @@ Entry encode(final T o, final String parentDN)
           throw new LDAPPersistException(
                ERR_OBJECT_HANDLER_ERROR_INVOKING_POST_ENCODE_METHOD.get(
                     postEncodeMethod.getName(), type.getName(),
-                    StaticUtils.getExceptionMessage(t)), t);
+                    StaticUtils.getExceptionMessage(ex)), ex);
         }
       }
     }
diff --git a/src/com/unboundid/util/StaticUtils.java b/src/com/unboundid/util/StaticUtils.java
index d95a227f4..0ae8e42fc 100644
--- a/src/com/unboundid/util/StaticUtils.java
+++ b/src/com/unboundid/util/StaticUtils.java
@@ -3245,4 +3245,99 @@ public static boolean isWithinUnitTest()
   {
     return IS_WITHIN_UNIT_TESTS;
   }
+
+
+
+  /**
+   * Throws an {@code Error} or a {@code RuntimeException} based on the provided
+   * {@code Throwable} object.  This method will always throw something,
+   * regardless of the provided {@code Throwable} object.
+   *
+   * @param  throwable  The {@code Throwable} object to use to create the
+   *                    exception to throw.
+   *
+   * @throws  Error  If the provided {@code Throwable} object is an
+   *                 {@code Error} instance, then that {@code Error} instance
+   *                 will be re-thrown.
+   *
+   * @throws  RuntimeException  If the provided {@code Throwable} object is a
+   *                            {@code RuntimeException} instance, then that
+   *                            {@code RuntimeException} instance will be
+   *                            re-thrown.  Otherwise, it must be a checked
+   *                            exception and that checked exception will be
+   *                            re-thrown as a {@code RuntimeException}.
+   */
+  public static void throwErrorOrRuntimeException(final Throwable throwable)
+         throws Error, RuntimeException
+  {
+    Validator.ensureNotNull(throwable);
+
+    if (throwable instanceof Error)
+    {
+      throw (Error) throwable;
+    }
+    else if (throwable instanceof RuntimeException)
+    {
+      throw (RuntimeException) throwable;
+    }
+    else
+    {
+      throw new RuntimeException(throwable);
+    }
+  }
+
+
+
+  /**
+   * Re-throws the provided {@code Throwable} instance only if it is an
+   * {@code Error} or a {@code RuntimeException} instance; otherwise, this
+   * method will return without taking any action.
+   *
+   * @param  throwable  The {@code Throwable} object to examine and potentially
+   *                    re-throw.
+   *
+   * @throws  Error  If the provided {@code Throwable} object is an
+   *                 {@code Error} instance, then that {@code Error} instance
+   *                 will be re-thrown.
+   *
+   * @throws  RuntimeException  If the provided {@code Throwable} object is a
+   *                            {@code RuntimeException} instance, then that
+   *                            {@code RuntimeException} instance will be
+   *                            re-thrown.
+   */
+  public static void rethrowIfErrorOrRuntimeException(final Throwable throwable)
+         throws Error, RuntimeException
+  {
+    if (throwable instanceof Error)
+    {
+      throw (Error) throwable;
+    }
+    else if (throwable instanceof RuntimeException)
+    {
+      throw (RuntimeException) throwable;
+    }
+  }
+
+
+
+  /**
+   * Re-throws the provided {@code Throwable} instance only if it is an
+   * {@code Error}; otherwise, this method will return without taking any
+   * action.
+   *
+   * @param  throwable  The {@code Throwable} object to examine and potentially
+   *                    re-throw.
+   *
+   * @throws  Error  If the provided {@code Throwable} object is an
+   *                 {@code Error} instance, then that {@code Error} instance
+   *                 will be re-thrown.
+   */
+  public static void rethrowIfError(final Throwable throwable)
+         throws Error
+  {
+    if (throwable instanceof Error)
+    {
+      throw (Error) throwable;
+    }
+  }
 }
diff --git a/src/com/unboundid/util/parallel/AsynchronousParallelProcessor.java b/src/com/unboundid/util/parallel/AsynchronousParallelProcessor.java
index 575f59a68..cf0d85b4e 100644
--- a/src/com/unboundid/util/parallel/AsynchronousParallelProcessor.java
+++ b/src/com/unboundid/util/parallel/AsynchronousParallelProcessor.java
@@ -31,6 +31,7 @@
 
 import com.unboundid.util.Debug;
 import com.unboundid.util.InternalUseOnly;
+import com.unboundid.util.StaticUtils;
 import com.unboundid.util.ThreadSafety;
 import com.unboundid.util.ThreadSafetyLevel;
 
@@ -169,7 +170,7 @@ public synchronized void submit(final I input)
     if (resultProcessingError != null)
     {
       shutdown();
-      throw new RuntimeException(resultProcessingError);
+      StaticUtils.throwErrorOrRuntimeException(resultProcessingError);
     }
 
     pendingQueue.put(input);
diff --git a/src/com/unboundid/util/parallel/ParallelProcessor.java b/src/com/unboundid/util/parallel/ParallelProcessor.java
index ccfc6aa62..8fe92cc14 100644
--- a/src/com/unboundid/util/parallel/ParallelProcessor.java
+++ b/src/com/unboundid/util/parallel/ParallelProcessor.java
@@ -345,7 +345,7 @@ private void processInParallel()
         outputs.set(next, process(input));
       }
     }
-    catch (final Throwable e)
+    catch (final Exception e)
     {
       Debug.debugException(e);
       // As with catching InterruptedException above, it's bad if this
diff --git a/tests/unit/src/com/unboundid/util/StaticUtilsTestCase.java b/tests/unit/src/com/unboundid/util/StaticUtilsTestCase.java
index c215fc9e2..3818f1a3a 100644
--- a/tests/unit/src/com/unboundid/util/StaticUtilsTestCase.java
+++ b/tests/unit/src/com/unboundid/util/StaticUtilsTestCase.java
@@ -2551,4 +2551,138 @@ public void testToArray()
     assertEquals(nonEmptyIntegerArray.length, 5);
     assertEquals(nonEmptyIntegerArray, new Integer[] { 1, 2, 3, 4, 5 });
   }
+
+
+
+  /**
+   * Tests the behavior of the {@code isWithinUnitTest} method.
+   *
+   * @throws  Exception  If an unexpected problem occurs.
+   */
+  @Test()
+  public void testisWithinUnitTest()
+         throws Exception
+  {
+    assertTrue(StaticUtils.isWithinUnitTest());
+  }
+
+
+
+  /**
+   * Tests the behavior of the {@code throwErrorOrRuntimeException} method.
+   *
+   * @throws  Exception  If an unexpected problem occurs.
+   */
+  @Test()
+  public void testThrowErrorOrRuntimeException()
+         throws Exception
+  {
+    try
+    {
+      StaticUtils.throwErrorOrRuntimeException(null);
+      fail("Expected an exception from a null throwable");
+    }
+    catch (final LDAPSDKUsageException e)
+    {
+      // This was expected.
+    }
+
+    try
+    {
+      StaticUtils.throwErrorOrRuntimeException(new UnknownError("Testing"));
+      fail("Expected an UnknownError from an UnknownError throwable");
+    }
+    catch (final UnknownError e)
+    {
+      // This was expected.
+    }
+
+    try
+    {
+      StaticUtils.throwErrorOrRuntimeException(
+           new NullPointerException("Testing"));
+      fail("Expected a NullPointerException from a NullPointerException " +
+           "throwable");
+    }
+    catch (final NullPointerException e)
+    {
+      // This was expected.
+    }
+
+    try
+    {
+      StaticUtils.throwErrorOrRuntimeException(new IOException("Testing"));
+      fail("Expected a RuntimeException from an IOException throwable");
+    }
+    catch (final RuntimeException e)
+    {
+      // This was expected.
+    }
+  }
+
+
+
+  /**
+   * Tests the behavior of the {@code rethrowIfErrorOrRuntimeException} method.
+   *
+   * @throws  Exception  If an unexpected problem occurs.
+   */
+  @Test()
+  public void testRethrowIfErrorOrRuntimeException()
+         throws Exception
+  {
+    StaticUtils.rethrowIfErrorOrRuntimeException(null);
+
+    try
+    {
+      StaticUtils.rethrowIfErrorOrRuntimeException(new UnknownError("Testing"));
+      fail("Expected an UnknownError from an UnknownError throwable");
+    }
+    catch (final UnknownError e)
+    {
+      // This was expected.
+    }
+
+    try
+    {
+      StaticUtils.rethrowIfErrorOrRuntimeException(
+           new NullPointerException("Testing"));
+      fail("Expected a NullPointerException from a NullPointerException " +
+           "throwable");
+    }
+    catch (final NullPointerException e)
+    {
+      // This was expected.
+    }
+
+    StaticUtils.rethrowIfErrorOrRuntimeException(new IOException("Testing"));
+  }
+
+
+
+  /**
+   * Tests the behavior of the {@code rethrowIfError} method.
+   *
+   * @throws  Exception  If an unexpected problem occurs.
+   */
+  @Test()
+  public void testRethrowIfError()
+         throws Exception
+  {
+    StaticUtils.rethrowIfError(null);
+
+    try
+    {
+      StaticUtils.rethrowIfError(new UnknownError("Testing"));
+      fail("Expected an UnknownError from an UnknownError throwable");
+    }
+    catch (final UnknownError e)
+    {
+      // This was expected.
+    }
+
+    StaticUtils.rethrowIfError(new NullPointerException("Testing"));
+
+    StaticUtils.rethrowIfError(new IOException("Testing"));
+  }
 }