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.
pingidentity · Aug 28, 2018 · 9b012d2 · 9b012d2
1 parent 1916616
commit 9b012d2
Show file tree

Hide file tree

Showing 12 changed files with 271 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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