This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit acc90182d5e93021876415d9d7895b09c4152642 Author: opencode <[email protected]> AuthorDate: Sun Nov 1 16:50:40 2026 +0100 Continue the class loader chain past broken visible copies ClassLoaderUtil.loadClass() caught only ClassNotFoundException per loader, so a copy visible to the first loader that failed to link or initialize - ExceptionInInitializerError from a static block, UnsupportedClassVersionError or NoClassDefFoundError from a stale jar - or was refused by a policy, aborted the documented fallback chain and escaped raw to callers that only plan for exceptions. The chain now continues past such a dead copy, warning so that resolving the name from a later loader is never silent, and the eventual failure is always the declared ClassNotFoundException with the last attempted failure as its cause. Fatal handling is the container's own: the per-loader catch takes any Throwable and calls ExceptionUtils.handleThrowable() - copied verbatim to pool.util because tomcat-jdbc.jar must not depend on catalina.jar - which propagates a dying JVM's VirtualMachineError unconverted and accepts the recoverable StackOverflowError as a dead copy. The helper is preloaded so it cannot fail to load inside the catch that needs it, and GenericNamingResourcesFactory adopts it in place of its hand-rolled equivalents. Supplying no class loaders now fails with a message that says so instead of one formatted around a null, and the broken-copy warning makes no claim about loaders it will not attempt. New TestClassLoaderUtil pins the real JVM shapes, including a class whose static initializer dies of a VirtualMachineError. --- .../jdbc/naming/GenericNamingResourcesFactory.java | 30 +-- .../apache/tomcat/jdbc/pool/ClassLoaderUtil.java | 85 +++++++- .../tomcat/jdbc/pool/util/ExceptionUtils.java | 77 +++++++ .../tomcat/jdbc/pool/util/LocalStrings.properties | 2 + .../tomcat/jdbc/pool/TestClassLoaderUtil.java | 233 +++++++++++++++++++++ 5 files changed, 399 insertions(+), 28 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java index 3b39793a22..5400b0702d 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/naming/GenericNamingResourcesFactory.java @@ -39,6 +39,7 @@ import javax.naming.spi.ObjectFactory; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.jdbc.pool.ClassLoaderUtil; +import org.apache.tomcat.jdbc.pool.util.ExceptionUtils; import org.apache.tomcat.jdbc.pool.util.MessageFactory; /** @@ -127,14 +128,10 @@ public class GenericNamingResourcesFactory implements ObjectFactory { ne.setRootCause(e); throw ne; } catch (InvocationTargetException e) { - // The class has a no-argument constructor, but it threw. Propagate - // only a fatal VirtualMachineError, in the spirit of - // ExceptionUtils.handleThrowable(), which tomcat-jdbc.jar cannot - // use directly. - Throwable cause = e.getCause(); - if (cause instanceof VirtualMachineError) { - throw (VirtualMachineError) cause; - } + // The class has a no-argument constructor, but it threw. + // ExceptionUtils.handleThrowable(e.getCause()) propagates a fatal + // VirtualMachineError and accepts everything else for reporting. + ExceptionUtils.handleThrowable(e.getCause()); NamingException ne = new NamingException(MessageFactory.get("genericNamingResourcesFactory.constructorThrew", type, name)); ne.setRootCause(e); @@ -408,18 +405,13 @@ public class GenericNamingResourcesFactory implements ObjectFactory { name, o.getClass().getName()), iae); return SetResult.REPORTED; } catch (InvocationTargetException ie) { - // In the spirit of ExceptionUtils.handleThrowable(), which is not - // available to tomcat-jdbc.jar: propagate the fatal - // VirtualMachineError from the setter and report everything else. - // The configured value is deliberately not echoed: a failing - // password setter would otherwise write the secret to the log. + // ExceptionUtils.handleThrowable() propagates the fatal + // VirtualMachineError from the setter and accepts everything + // else for reporting. The configured value is deliberately not + // echoed: a failing password setter would otherwise write the + // secret to the log. Throwable cause = ie.getCause(); - if (cause instanceof ThreadDeath) { - throw (ThreadDeath) cause; - } - if (cause instanceof VirtualMachineError) { - throw (VirtualMachineError) cause; - } + ExceptionUtils.handleThrowable(cause); log.warn(MessageFactory.get("genericNamingResourcesFactory.setterThrew", name, o.getClass().getName()), ie); return SetResult.REPORTED; diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ClassLoaderUtil.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ClassLoaderUtil.java index e425b2d60a..dd31d43ac3 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ClassLoaderUtil.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ClassLoaderUtil.java @@ -19,6 +19,7 @@ package org.apache.tomcat.jdbc.pool; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.jdbc.pool.util.ExceptionUtils; import org.apache.tomcat.jdbc.pool.util.MessageFactory; /** @@ -27,6 +28,17 @@ import org.apache.tomcat.jdbc.pool.util.MessageFactory; public class ClassLoaderUtil { private static final Log log = LogFactory.getLog(ClassLoaderUtil.class); + static { + // handleThrowable() first runs inside a catch block below; load and + // initialize the class up front so a failure to load it cannot mask + // the error it is called to handle. + ExceptionUtils.preload(); + } + + /** + * The kill switch is sampled once at class initialization: setting the + * property after the pool classes have loaded has no effect. + */ private static final boolean onlyAttemptFirstLoader = Boolean .getBoolean("org.apache.tomcat.jdbc.pool.onlyAttemptCurrentClassLoader"); @@ -38,16 +50,35 @@ public class ClassLoaderUtil { /** * Attempts to load the specified class using the given class loaders in order. + * <p> + * The chain continues past a loader that cannot see the class at all, and + * also past a loader that sees a copy which fails to link or initialize: + * the name may still resolve to a working copy further along the chain, + * as when a stale jar sits in the container's class path while the + * working copy ships elsewhere. A loader that sees a broken copy is + * recorded in the failure, and the fallback is logged as a warning, so + * loading the name from a later loader never happens silently. The fatal + * errors that {@code ExceptionUtils.handleThrowable} propagates - a + * dying JVM's {@code VirtualMachineError} - abort the chain unconverted; + * every other failure of a visible copy is a dead copy, including the + * JVM's standing "Could not initialize class" marker for a class whose + * initialization already failed. * * @param className the fully qualified class name * @param classLoaders the class loaders to attempt, in order * * @return the loaded class * - * @throws ClassNotFoundException if the class cannot be found by any of the loaders + * @throws ClassNotFoundException if no class loader was supplied, or none + * of them could load the class; the failure of the last loader + * attempted is the cause */ public static Class<?> loadClass(String className, ClassLoader... classLoaders) throws ClassNotFoundException { - ClassNotFoundException last = null; + if (classLoaders == null || classLoaders.length == 0) { + throw new ClassNotFoundException( + MessageFactory.get("classLoaderUtil.classLoadersNone", className)); + } + Throwable last = null; StringBuilder errorMsg = null; for (ClassLoader cl : classLoaders) { try { @@ -61,13 +92,27 @@ public class ClassLoaderUtil { } } catch (ClassNotFoundException x) { last = x; - if (errorMsg == null) { - errorMsg = new StringBuilder(); - } else { - errorMsg.append(';'); - } - errorMsg.append("ClassLoader:"); - errorMsg.append(cl); + errorMsg = appendFailure(errorMsg, cl, null); + } catch (Throwable x) { + // ExceptionUtils.handleThrowable() propagates the fatal + // errors - a VirtualMachineError from a dying JVM - and + // swallows the recoverable ones; it is the same treatment + // the container gives a Throwable before acting on it. + ExceptionUtils.handleThrowable(x); + // The class is visible to this loader but the copy is broken: + // it fails to link (class file version, missing dependency), + // its static initializer throws, or a policy refused it. Only + // the not-found case used to continue the chain, so a broken + // copy in the first loader masked a working copy in a later + // one and a raw Error or RuntimeException escaped to callers + // that only plan for ClassNotFoundException. Record the + // failure and attempt the remaining loaders: the JVM's + // standing "Could not initialize class" marker for a class + // whose initialization already failed carries no fatal error + // to rethrow and is as dead a copy as any. + last = x; + errorMsg = appendFailure(errorMsg, cl, x); + log.warn(MessageFactory.get("classLoaderUtil.classLoadError", className, cl), x); } if (onlyAttemptFirstLoader) { break; @@ -77,5 +122,27 @@ public class ClassLoaderUtil { last); } + /** + * Record one loader's failure for the summary message of the eventual + * ClassNotFoundException. A not-found failure (no reason given) keeps the + * historical format, which is just the loader's identity; a broken-copy + * failure names it too, so the operator can tell a dead copy from an + * absent class in the message alone - the failure's own stack trace, + * carried to the log with the warning, shows its cause. + */ + private static StringBuilder appendFailure(StringBuilder errorMsg, ClassLoader cl, Throwable failure) { + if (errorMsg == null) { + errorMsg = new StringBuilder(); + } else { + errorMsg.append(';'); + } + errorMsg.append("ClassLoader:"); + errorMsg.append(cl); + if (failure != null) { + errorMsg.append(" -> ").append(failure); + } + return errorMsg; + } + } diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/ExceptionUtils.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/ExceptionUtils.java new file mode 100644 index 0000000000..667eea8dce --- /dev/null +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/ExceptionUtils.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.jdbc.pool.util; + +import java.lang.reflect.InvocationTargetException; + + +/** + * Utilities for handling Throwables and Exceptions. Copy of + * <code>org.apache.tomcat.util.ExceptionUtils</code>, kept verbatim apart from + * the package declaration, because tomcat-jdbc.jar must not depend on + * catalina.jar. + */ +public class ExceptionUtils { + + /** + * Default constructor. This class is not intended to be instantiated. + */ + public ExceptionUtils() { + // NO-OP + } + + /** + * Checks whether the supplied Throwable is one that needs to be rethrown and swallows all others. + * + * @param t the Throwable to check + */ + public static void handleThrowable(Throwable t) { + if (t instanceof StackOverflowError) { + // Swallow silently - it should be recoverable + return; + } + if (t instanceof VirtualMachineError) { + throw (VirtualMachineError) t; + } + // All other instances of Throwable will be silently swallowed + } + + /** + * Checks whether the supplied Throwable is an instance of <code>InvocationTargetException</code> and returns the + * throwable that is wrapped by it, if there is any. + * + * @param t the Throwable to check + * + * @return <code>t</code> or <code>t.getCause()</code> + */ + public static Throwable unwrapInvocationTargetException(Throwable t) { + if (t instanceof InvocationTargetException && t.getCause() != null) { + return t.getCause(); + } + return t; + } + + + /** + * NO-OP method provided to enable simple preloading of this class. Since the class is used extensively in error + * handling, it is prudent to preload it to avoid any failure to load this class masking the true problem during + * error handling. + */ + public static void preload() { + // NO-OP + } +} diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties index c83f0a47f1..5739b5bd88 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties @@ -24,6 +24,8 @@ abstractQueryReport.unableToProcessSlowQuery=Unable to process slow query for {0 classLoaderUtil.classLoaderNull=Classloader is null classLoaderUtil.classLoadFailed=Unable to load class: {0} from {1} +classLoaderUtil.classLoadError=Unable to load class [{0}] using class loader [{1}]. +classLoaderUtil.classLoadersNone=Unable to load class [{0}]: no class loaders supplied connectionPool.checkAbandonedFailed=checkAbandoned failed, it will be retried. connectionPool.checkIdleFailed=checkIdle failed, it will be retried. diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestClassLoaderUtil.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestClassLoaderUtil.java new file mode 100644 index 0000000000..63531c526f --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestClassLoaderUtil.java @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.jdbc.pool; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link ClassLoaderUtil}. The load chain must fall through a + * loader that cannot see the class, and also past a loader that sees a copy + * which fails to link or initialize, so that a broken copy in an early + * loader cannot mask a working copy in a later one. When nothing loads, the + * declared failure type is {@code ClassNotFoundException} regardless of how + * each individual loader failed, and a fatal {@code VirtualMachineError} + * from a static initializer propagates unconverted. + */ +class TestClassLoaderUtil { + + /** + * A loader that stands for a stale or partially deployed jar: the class + * is visible to it, but attempting to load it fails with the given + * error, the way an unusable copy's linkage or initialization fails. + */ + private static final class BrokenCopyLoader extends ClassLoader { + + private final String target; + private final Error failure; + + BrokenCopyLoader(String target, Error failure) { + super(ClassLoader.getSystemClassLoader()); + this.target = target; + this.failure = failure; + } + + @Override + protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (target.equals(name)) { + throw failure; + } + return super.loadClass(name, resolve); + } + } + + /** + * A loader that stands for a security policy refusing the load. + */ + private static final class DeniedLoader extends ClassLoader { + + private final String target; + + DeniedLoader(String target) { + super(ClassLoader.getSystemClassLoader()); + this.target = target; + } + + @Override + protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (target.equals(name)) { + throw new SecurityException("policy denied"); + } + return super.loadClass(name, resolve); + } + } + + /** + * A copy that is visible to the first loader but throws from its static + * initializer - the classic stale JDBC driver jar - must not prevent the + * second loader's working copy from being used. + */ + @Test + void brokenCopyInFirstLoaderFallsBackToSecondLoader() throws Exception { + ClassLoader broken = new BrokenCopyLoader("java.sql.Driver", + new ExceptionInInitializerError(new RuntimeException("broken stale copy"))); + Class<?> loaded = ClassLoaderUtil.loadClass("java.sql.Driver", + broken, ClassLoader.getSystemClassLoader()); + Assertions.assertEquals(java.sql.Driver.class, loaded); + } + + /** + * A policy refusal from the first loader is likewise not fatal to the + * chain. + */ + @Test + void securityExceptionInFirstLoaderFallsBackToSecondLoader() throws Exception { + ClassLoader denied = new DeniedLoader("java.sql.Driver"); + Class<?> loaded = ClassLoaderUtil.loadClass("java.sql.Driver", + denied, ClassLoader.getSystemClassLoader()); + Assertions.assertEquals(java.sql.Driver.class, loaded); + } + + /** + * When every loader fails, the caller gets the declared + * ClassNotFoundException even though no loader actually reported one, + * with the linkage failure as the cause and named in the message, where + * the historical not-found format lists only the loaders. + */ + @Test + void allLoadersBrokenStillThrowsTheDeclaredContract() { + ClassLoader broken = new BrokenCopyLoader("java.sql.Driver", + new ExceptionInInitializerError(new RuntimeException("broken stale copy"))); + ClassNotFoundException cnfe = Assertions.assertThrows(ClassNotFoundException.class, + () -> ClassLoaderUtil.loadClass("java.sql.Driver", broken)); + Assertions.assertInstanceOf(LinkageError.class, cnfe.getCause()); + Assertions.assertTrue(cnfe.getMessage().contains("ExceptionInInitializerError"), + () -> "message should name the failure, but was: " + cnfe.getMessage()); + } + + /** + * A class whose static initializer dies of a VirtualMachineError, used to + * test the real JVM shapes: the error raw on the first initialization + * attempt, the unlinked "Could not initialize class" marker on every + * attempt after that. + */ + static class FatalInit { + + private static final Object NOT_A_CONSTANT = new Object(); + + static { + // Condition is not a compile-time constant, so javac accepts the + // initializer; at runtime the fatal error always wins. + if (NOT_A_CONSTANT != null) { + throw new OutOfMemoryError("fatal-from-initializer"); + } + } + } + + /** + * A VirtualMachineError that reaches the load attempt itself - a dying + * JVM - must abort the chain unconverted: the working copy in the second + * loader must not be reached. + */ + @Test + void virtualMachineErrorFromALoaderAbortsTheChain() { + ClassLoader fatal = new BrokenCopyLoader("java.sql.Driver", + new OutOfMemoryError("fatal")); + OutOfMemoryError thrown = Assertions.assertThrows(OutOfMemoryError.class, + () -> ClassLoaderUtil.loadClass("java.sql.Driver", fatal, + ClassLoader.getSystemClassLoader())); + Assertions.assertEquals("fatal", thrown.getMessage()); + } + + /** + * A StackOverflowError is recoverable and ExceptionUtils.handleThrowable + * deliberately swallows it: the attempt is then an ordinary dead copy and + * the fallback must proceed. + */ + @Test + void stackOverflowErrorFallsBackAsADeadCopy() throws Exception { + ClassLoader overflowed = new BrokenCopyLoader("java.sql.Driver", + new StackOverflowError()); + Class<?> loaded = ClassLoaderUtil.loadClass("java.sql.Driver", + overflowed, ClassLoader.getSystemClassLoader()); + Assertions.assertEquals(java.sql.Driver.class, loaded); + } + + /** + * The shape the JVM really produces: the first initialization attempt of + * a class dies of a VirtualMachineError and propagates it unconverted to + * the caller that made the attempt. Every attempt after that sees the + * JVM's standing "Could not initialize class" NoClassDefFoundError, + * which carries no fatal error to rethrow and is indistinguishable from + * any other dead copy - exactly the case the documented fallback exists + * for. The later attempt therefore ends in the declared + * ClassNotFoundException, naming the dead copy. + */ + @Test + void virtualMachineErrorPropagatesOnFirstAttemptAndDeadCopyOnLaterOnes() { + ClassLoader loader = TestClassLoaderUtil.class.getClassLoader(); + String name = FatalInit.class.getName(); + OutOfMemoryError first = Assertions.assertThrows(OutOfMemoryError.class, + () -> ClassLoaderUtil.loadClass(name, loader, loader)); + Assertions.assertEquals("fatal-from-initializer", first.getMessage()); + ClassNotFoundException later = Assertions.assertThrows(ClassNotFoundException.class, + () -> ClassLoaderUtil.loadClass(name, loader, loader)); + Assertions.assertTrue(later.getMessage().contains("Could not initialize class"), + () -> "message should name the dead copy, but was: " + later.getMessage()); + } + + /** + * Regression: a class no loader can see still ends in a + * ClassNotFoundException reporting both loaders, with the last loader's + * not-found failure as the cause. + */ + @Test + void notFoundByEveryLoaderEndsInClassNotFound() { + ClassNotFoundException cnfe = Assertions.assertThrows(ClassNotFoundException.class, + () -> ClassLoaderUtil.loadClass("com.example.NotThere", + ClassLoader.getSystemClassLoader(), ClassLoader.getSystemClassLoader())); + Assertions.assertInstanceOf(ClassNotFoundException.class, cnfe.getCause()); + Assertions.assertTrue(cnfe.getMessage().contains("com.example.NotThere"), + () -> "message should name the class, but was: " + cnfe.getMessage()); + } + + /** + * Regression: a not-found in the first loader falls through to the + * second, and a null loader element is skipped rather than fatal. + */ + @Test + void notFoundAndNullLoadersSkipToTheNextLoader() throws Exception { + Class<?> loaded = ClassLoaderUtil.loadClass("java.sql.Driver", + null, ClassLoader.getSystemClassLoader()); + Assertions.assertEquals(java.sql.Driver.class, loaded); + } + + /** + * With nothing to attempt, the caller gets the declared + * ClassNotFoundException naming the missing supply, not an exception + * formatted around a null. + */ + @Test + void withoutAnyClassLoadersTheFailureSaysSo() { + ClassNotFoundException empty = Assertions.assertThrows(ClassNotFoundException.class, + () -> ClassLoaderUtil.loadClass("java.sql.Driver")); + Assertions.assertTrue(empty.getMessage().contains("no class loaders supplied"), + () -> "message should say nothing was supplied, but was: " + empty.getMessage()); + Assertions.assertThrows(ClassNotFoundException.class, + () -> ClassLoaderUtil.loadClass("java.sql.Driver", (ClassLoader[]) null)); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
