From e986065b7aca0ad7a7fd851af67c2acb47c10dd5 Mon Sep 17 00:00:00 2001
From: Thomas Neidhart <tn@apache.org>
Date: Fri, 22 Feb 2013 14:49:22 +0000
Subject: [PATCH] [LOGGING-144] Do not swallow certain Errors anymore, like
 ThreadDeath and VirtualMachineError.

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/logging/trunk@1449064 13f79535-47bb-0310-9956-ffa450edef68
---
 .../apache/commons/logging/LogFactory.java    | 26 ++++++++++++++++++-
 .../commons/logging/impl/LogFactoryImpl.java  | 14 ++++++----
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/apache/commons/logging/LogFactory.java b/src/main/java/org/apache/commons/logging/LogFactory.java
index e0fa6ea..30a3dbe 100644
--- a/src/main/java/org/apache/commons/logging/LogFactory.java
+++ b/src/main/java/org/apache/commons/logging/LogFactory.java
@@ -333,6 +333,8 @@ public abstract class LogFactory {
             Class implementationClass = Class.forName(storeImplementationClass);
             result = (Hashtable) implementationClass.newInstance();
         } catch (Throwable t) {
+            handleThrowable(t); // may re-throw t
+
             // ignore
             if (!WEAK_HASHTABLE_CLASSNAME.equals(storeImplementationClass)) {
                 // if the user's trying to set up a custom implementation, give a clue
@@ -362,6 +364,28 @@ public abstract class LogFactory {
         return src.trim();
     }
 
+    /**
+     * Checks whether the supplied Throwable is one that needs to be
+     * re-thrown and ignores all others.
+     *
+     * The following errors are re-thrown:
+     * <ul>
+     *   <li>ThreadDeath</li>
+     *   <li>VirtualMachineError</li>
+     * </ul>
+     *
+     * @param t the Throwable to check
+     */
+    protected static void handleThrowable(Throwable t) {
+        if (t instanceof ThreadDeath) {
+            throw (ThreadDeath) t;
+        }
+        if (t instanceof VirtualMachineError) {
+            throw (VirtualMachineError) t;
+        }
+        // All other instances of Throwable will be silently ignored
+    }
+
     /**
      * Construct (if necessary) and return a <code>LogFactory</code>
      * instance, using the following ordered lookup procedure to determine
@@ -1337,7 +1361,7 @@ public abstract class LogFactory {
                         if (stream != null) {
                             try {
                                 stream.close();
-                            } catch (Throwable t) {
+                            } catch (IOException e) {
                                 // ignore exception; this should not happen
                                 if (isDiagnosticsEnabled()) {
                                     logDiagnostic("Unable to close stream for URL " + url);
diff --git a/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java b/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java
index 7d4d545..56b4a14 100644
--- a/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java
+++ b/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java
@@ -425,7 +425,7 @@ public class LogFactoryImpl extends LogFactory {
             } else {
                 classLoaderName = objectId(classLoader);
             }
-        } catch(SecurityException e) {
+        } catch (SecurityException e) {
             classLoaderName = "UNKNOWN";
         }
         diagnosticPrefix = "[LogFactoryImpl@" + System.identityHashCode(this) + " from " + classLoaderName + "] ";
@@ -565,6 +565,7 @@ public class LogFactoryImpl extends LogFactory {
             Throwable c = e.getTargetException();
             throw new LogConfigurationException(c == null ? e : c);
         } catch (Throwable t) {
+            handleThrowable(t); // may re-throw t
             // A problem occurred invoking the Constructor or Method
             // previously discovered
             throw new LogConfigurationException(t);
@@ -635,7 +636,7 @@ public class LogFactoryImpl extends LogFactory {
                             return cl.getParent();
                         }
                     });
-        } catch(SecurityException ex) {
+        } catch (SecurityException ex) {
             logDiagnostic("[SECURITY] Unable to obtain parent classloader");
             return null;
         }
@@ -668,7 +669,7 @@ public class LogFactoryImpl extends LogFactory {
                 }
                 return true;
             }
-        } catch(LogConfigurationException e) {
+        } catch (LogConfigurationException e) {
             if (isDiagnosticsEnabled()) {
                 logDiagnostic("Logging system '" + name + "' is available but not useable.");
             }
@@ -1067,11 +1068,12 @@ public class LogFactoryImpl extends LogFactory {
                               "' is unable to initialize itself when loaded via classloader " + objectId(currentCL) +
                               ": " + msg.trim());
                 break;
-            } catch(LogConfigurationException e) {
+            } catch (LogConfigurationException e) {
                 // call to handleFlawedHierarchy above must have thrown
                 // a LogConfigurationException, so just throw it on
                 throw e;
-            } catch(Throwable t) {
+            } catch (Throwable t) {
+                handleThrowable(t); // may re-throw t
                 // handleFlawedDiscovery will determine whether this is a fatal
                 // problem or not. If it is fatal, then a LogConfigurationException
                 // will be thrown.
@@ -1097,6 +1099,7 @@ public class LogFactoryImpl extends LogFactory {
                 this.logMethod = logAdapterClass.getMethod("setLogFactory", logMethodSignature);
                 logDiagnostic("Found method setLogFactory(LogFactory) in '" + logAdapterClassName + "'");
             } catch (Throwable t) {
+                handleThrowable(t); // may re-throw t
                 this.logMethod = null;
                 logDiagnostic("[INFO] '" + logAdapterClassName + "' from classloader " + objectId(currentCL) +
                               " does not declare optional method " + "setLogFactory(LogFactory)");
@@ -1337,6 +1340,7 @@ public class LogFactoryImpl extends LogFactory {
                                   objectId(badClassLoader) + ". It is bound to a Log interface which is not" +
                                   " the one loaded from classloader " + objectId(logInterfaceClassLoader));
                 } catch (Throwable t) {
+                    handleThrowable(t); // may re-throw t
                     logDiagnostic("Error while trying to output diagnostics about" + " bad class '" + badClass + "'");
                 }
             }