diff --git a/catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/ClassLoaderResourceCleanerUtils.java b/catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/ClassLoaderResourceCleanerUtils.java index 6a91f4ac484..b4c33ffa8a9 100644 --- a/catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/ClassLoaderResourceCleanerUtils.java +++ b/catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/ClassLoaderResourceCleanerUtils.java @@ -20,10 +20,20 @@ package org.apache.gravitino.utils; import com.google.common.annotations.VisibleForTesting; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.lang.ref.Reference; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.security.Provider; +import java.security.Security; +import java.util.Collection; import java.util.IdentityHashMap; +import java.util.ResourceBundle; import java.util.Timer; import java.util.concurrent.ScheduledExecutorService; +import javax.annotation.Nullable; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.commons.lang3.reflect.MethodUtils; import org.slf4j.Logger; @@ -70,6 +80,16 @@ public static void closeClassLoaderResource(ClassLoader classLoader) { // instance. executeAndCatch(ClassLoaderResourceCleanerUtils::releaseLogFactoryInCommonLogging, classLoader); + executeAndCatch(ClassLoaderResourceCleanerUtils::removeLoggerContextListeners, classLoader); + + executeAndCatch(ClassLoaderResourceCleanerUtils::deregisterJdbcDrivers, classLoader); + + executeAndCatch(ClassLoaderResourceCleanerUtils::shutdownMysqlConnectionCleanup, classLoader); + + executeAndCatch(ClassLoaderResourceCleanerUtils::removeSecurityProviders, classLoader); + + executeAndCatch(ClassLoaderResourceCleanerUtils::clearResourceBundleCache, classLoader); + executeAndCatch(ClassLoaderResourceCleanerUtils::closeResourceInAWS, classLoader); executeAndCatch(ClassLoaderResourceCleanerUtils::closeResourceInGCP, classLoader); @@ -160,8 +180,37 @@ private static void stopThreadsAndClearThreadLocalVariables(ClassLoader classLoa } } - private static boolean runningWithClassLoader(Thread thread, ClassLoader targetClassLoader) { - return thread != null && thread.getContextClassLoader() == targetClassLoader; + /** + * Whether the thread belongs to the class loader being released. + * + *

The context ClassLoader is only one of the ways a thread can carry a catalog. A driver that + * starts its own housekeeping thread, such as PostgreSQL's {@code LazyCleaner}, is running code + * defined by the catalog's loader: the thread is a GC root, so its class alone keeps the loader + * alive no matter what its context ClassLoader says. + * + *

Ownership has to be read from the thread itself, never from what it happens to be running: a + * request thread executing an operation on this very catalog is not the catalog's to stop, and + * interrupting it fails the request with "Thread was interrupted while waiting for lock". + */ + @VisibleForTesting + static boolean runningWithClassLoader(Thread thread, ClassLoader targetClassLoader) { + if (thread == null) { + return false; + } + if (thread.getContextClassLoader() == targetClassLoader + || thread.getClass().getClassLoader() == targetClassLoader) { + return true; + } + try { + Object runnable = FieldUtils.readField(thread, "target", true); + if (runnable != null && runnable.getClass().getClassLoader() == targetClassLoader) { + return true; + } + } catch (Exception e) { + LOG.debug("Cannot read the runnable of thread {}", thread.getName(), e); + } + + return false; } private static Thread[] getAllThreads() { @@ -178,8 +227,9 @@ private static Thread[] getAllThreads() { return threads; } - private static void clearThreadLocalMap(Thread thread, ClassLoader targetClassLoader) { - if (thread == null || !thread.getName().startsWith("Gravitino-webserver-")) { + @VisibleForTesting + static void clearThreadLocalMap(Thread thread, ClassLoader targetClassLoader) { + if (thread == null) { return; } @@ -197,9 +247,10 @@ private static void clearThreadLocalMap(Thread thread, ClassLoader targetClassLo for (Object entry : table) { if (entry != null) { Object value = FieldUtils.readField(entry, "value", true); - if (value != null - && value.getClass().getClassLoader() != null - && value.getClass().getClassLoader() == targetClassLoader) { + // The entry is a WeakReference to the ThreadLocal itself, which can be the leaking + // side when the ThreadLocal was declared by a class of the dying catalog. + Object key = entry instanceof Reference ? ((Reference) entry).get() : null; + if (definedBy(value, targetClassLoader) || definedBy(key, targetClassLoader)) { LOG.debug( "Cleaning up thread local {} for thread {} with custom class loader", value, @@ -214,6 +265,31 @@ private static void clearThreadLocalMap(Thread thread, ClassLoader targetClassLo } } + /** + * Whether {@code value}, or what it refers to when it is a {@link Reference}, was defined by + * {@code classLoader}. + * + *

Looking through a {@link Reference} matters: caches such as Jackson's {@code BufferRecycler} + * park a {@code SoftReference} in a {@link ThreadLocal}. The reference itself is a bootstrap + * class, so only its referent identifies the owning catalog. Left in place, such an entry keeps + * the catalog's ClassLoader alive until heap pressure clears the soft reference, which Metaspace + * pressure alone never triggers. + */ + @VisibleForTesting + static boolean definedBy(@Nullable Object value, ClassLoader classLoader) { + if (value == null) { + return false; + } + if (value.getClass().getClassLoader() == classLoader) { + return true; + } + if (value instanceof Reference) { + Object referent = ((Reference) value).get(); + return referent != null && referent.getClass().getClassLoader() == classLoader; + } + return false; + } + /** * Clear shutdown hooks registered by the target class loader to prevent memory leaks. * @@ -236,6 +312,145 @@ private static void clearShutdownHooks(ClassLoader targetClassLoader) throws Exc }); } + /** + * Removes shutdown listeners the class loader registered on the shared Log4j {@code + * LoggerContext}. + * + *

commons-logging's {@code Log4jApiLogFactory} registers a {@code LogAdapter} with the + * LoggerContext of the server, which outlives every catalog. {@code LogFactory.release} drops the + * factory from its own cache but leaves that registration in place, so the adapter's class, and + * through it the catalog's ClassLoader, stays reachable from a static for the life of the + * process. + */ + /** + * Drops the {@link ResourceBundle} cache entries loaded through this class loader. + * + *

{@link ResourceBundle} caches bundles in a JVM-wide static map, behind soft references. A + * driver that loads message bundles, such as Oracle's {@code ErrorMessages}, therefore leaves its + * class - and the catalog's ClassLoader - reachable until heap pressure clears the soft + * reference, which Metaspace pressure alone never causes. + */ + @VisibleForTesting + static void clearResourceBundleCache(ClassLoader targetClassLoader) { + ResourceBundle.clearCache(targetClassLoader); + } + + /** + * Removes the JCA security providers the class loader installed. + * + *

{@link Security} keeps installed providers in a JVM-wide static list. Hadoop's cloud + * connectors install one, such as the shaded {@code OpenSSLProvider} that ships in the AWS + * bundle, and it is never removed, so the provider's class holds the catalog's loader for the + * life of the process. + */ + @VisibleForTesting + static void removeSecurityProviders(ClassLoader targetClassLoader) { + for (Provider provider : Security.getProviders()) { + if (provider.getClass().getClassLoader() == targetClassLoader) { + Security.removeProvider(provider.getName()); + LOG.info("Removed security provider {} of a released catalog ClassLoader", provider); + } + } + } + + /** + * Shuts down MySQL Connector/J's abandoned-connection cleanup thread when the driver belongs to + * this class loader. + * + *

The driver keeps that thread and its executor in a static field, and the executor's thread + * factory is a lambda defined by the catalog's loader, so a running cleanup thread pins the + * loader through its own stack frame. Connector/J exposes {@code uncheckedShutdown()} for exactly + * this case. + */ + private static void shutdownMysqlConnectionCleanup(ClassLoader targetClassLoader) + throws Exception { + Class cleanupThreadClass = + Class.forName( + "com.mysql.cj.jdbc.AbandonedConnectionCleanupThread", true, targetClassLoader); + if (!isOwnedByClassLoader(cleanupThreadClass, targetClassLoader)) { + LOG.debug( + "MySQL Connector/J is owned by {}, not {}; skipping shared-class cleanup", + cleanupThreadClass.getClassLoader(), + targetClassLoader); + return; + } + // uncheckedShutdown stops the thread even when the driver still believes it is in use, which + // is what unloading the ClassLoader requires; checkedShutdown returns without doing anything. + MethodUtils.invokeStaticMethod(cleanupThreadClass, "uncheckedShutdown"); + LOG.info("Shut down the MySQL abandoned-connection cleanup thread of a released ClassLoader"); + } + + /** + * Deregisters the JDBC drivers the class loader registered with {@link java.sql.DriverManager}. + * + *

{@code DriverManager} keeps registered drivers in a static list, and a driver defined by a + * catalog's ClassLoader keeps that loader alive for the life of the process. It cannot be removed + * from here directly: {@code DriverManager} filters both {@code getDrivers()} and {@code + * deregisterDriver()} by the class loader of the calling class, so from the server's ClassLoader + * the catalog's drivers are not even visible. Defining {@link JdbcDriverDeregisterer} inside the + * target loader and calling it there gives {@code DriverManager} a caller that owns them. + */ + @VisibleForTesting + static void deregisterJdbcDrivers(ClassLoader targetClassLoader) throws Exception { + String name = JdbcDriverDeregisterer.class.getName(); + byte[] bytecode; + try (InputStream in = + ClassLoaderResourceCleanerUtils.class + .getClassLoader() + .getResourceAsStream(name.replace('.', '/') + ".class")) { + if (in == null) { + LOG.debug("Cannot locate the bytecode of {}, skipping JDBC driver cleanup", name); + return; + } + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + byte[] chunk = new byte[8192]; + int read; + while ((read = in.read(chunk)) != -1) { + buffer.write(chunk, 0, read); + } + bytecode = buffer.toByteArray(); + } + + Method defineClass = + ClassLoader.class.getDeclaredMethod( + "defineClass", String.class, byte[].class, int.class, int.class); + defineClass.setAccessible(true); + Class deregisterer; + try { + deregisterer = + (Class) defineClass.invoke(targetClassLoader, name, bytecode, 0, bytecode.length); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof LinkageError) { + // Already defined by an earlier cleanup of the same loader, whose drivers are gone. + LOG.debug("{} is already defined in {}", name, targetClassLoader); + return; + } + throw e; + } + + Object deregistered = MethodUtils.invokeStaticMethod(deregisterer, "deregisterAll"); + if (deregistered instanceof Collection && !((Collection) deregistered).isEmpty()) { + LOG.info("Deregistered JDBC driver(s) {} of a released catalog ClassLoader", deregistered); + } + } + + @VisibleForTesting + static void removeLoggerContextListeners(ClassLoader targetClassLoader) throws Exception { + Class logManagerClass = Class.forName("org.apache.logging.log4j.LogManager"); + Object contextFactory = MethodUtils.invokeStaticMethod(logManagerClass, "getFactory"); + Object selector = MethodUtils.invokeMethod(contextFactory, "getSelector"); + Collection contexts = + (Collection) MethodUtils.invokeMethod(selector, "getLoggerContexts"); + for (Object context : contexts) { + Collection listeners = (Collection) FieldUtils.readField(context, "listeners", true); + if (listeners != null) { + listeners.removeIf( + listener -> + listener != null && listener.getClass().getClassLoader() == targetClassLoader); + } + } + } + /** * Release the LogFactory for the target class loader to prevent memory leaks. * diff --git a/catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/JdbcDriverDeregisterer.java b/catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/JdbcDriverDeregisterer.java new file mode 100644 index 00000000000..824281b4dac --- /dev/null +++ b/catalogs/catalog-common/src/main/java/org/apache/gravitino/utils/JdbcDriverDeregisterer.java @@ -0,0 +1,65 @@ +/* + * 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.gravitino.utils; + +import java.sql.Driver; +import java.sql.DriverManager; +import java.util.ArrayList; +import java.util.Enumeration; +import java.util.List; + +/** + * Deregisters the JDBC drivers a catalog's ClassLoader registered with {@link DriverManager}. + * + *

This class is never called through its own name. {@link DriverManager} filters both {@code + * getDrivers()} and {@code deregisterDriver()} by the class loader of the calling class, so a + * driver defined by a catalog's isolated ClassLoader is invisible, and undeletable, from the server + * ClassLoader. {@link ClassLoaderResourceCleanerUtils} therefore defines a copy of this class + * inside the catalog's ClassLoader and invokes it reflectively, so that {@code DriverManager} sees + * a caller that owns the drivers. Keep its dependencies to {@code java.*} only: the copy is defined + * directly from bytecode and resolves everything through the catalog's ClassLoader. + */ +public final class JdbcDriverDeregisterer { + + private JdbcDriverDeregisterer() {} + + /** + * Deregisters every driver defined by the ClassLoader of this class. + * + * @return the names of the drivers that were deregistered + */ + public static List deregisterAll() { + ClassLoader owner = JdbcDriverDeregisterer.class.getClassLoader(); + List deregistered = new ArrayList<>(); + Enumeration drivers = DriverManager.getDrivers(); + while (drivers.hasMoreElements()) { + Driver driver = drivers.nextElement(); + if (driver.getClass().getClassLoader() == owner) { + try { + DriverManager.deregisterDriver(driver); + deregistered.add(driver.getClass().getName()); + } catch (Exception e) { + // Leave the driver registered rather than failing the whole cleanup. + } + } + } + return deregistered; + } +} diff --git a/catalogs/catalog-common/src/test/java/org/apache/gravitino/utils/TestClassLoaderResourceCleanerUtils.java b/catalogs/catalog-common/src/test/java/org/apache/gravitino/utils/TestClassLoaderResourceCleanerUtils.java index c5a241d8c7a..52e4d10b6af 100644 --- a/catalogs/catalog-common/src/test/java/org/apache/gravitino/utils/TestClassLoaderResourceCleanerUtils.java +++ b/catalogs/catalog-common/src/test/java/org/apache/gravitino/utils/TestClassLoaderResourceCleanerUtils.java @@ -19,15 +19,169 @@ package org.apache.gravitino.utils; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.lang.ref.SoftReference; +import java.lang.ref.WeakReference; import java.net.URL; import java.net.URLClassLoader; +import java.security.Security; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; class TestClassLoaderResourceCleanerUtils { + private static final ThreadLocal SOFT_HOLDER = new ThreadLocal<>(); + private static final ThreadLocal UNRELATED_HOLDER = new ThreadLocal<>(); + + /** A class with no dependencies beyond java.*, so a bare-bones child loader can define it. */ + public static class Leaky {} + + /** A Runnable the child loader can define, standing in for a driver's housekeeping task. */ + public static class LeakyTask implements Runnable { + @Override + public void run() { + try { + Thread.sleep(60_000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + + private static URLClassLoader childLoaderOwning(Class clazz) throws Exception { + URL location = clazz.getProtectionDomain().getCodeSource().getLocation(); + // A null parent keeps delegation off the app loader, so the child defines the class itself. + return new URLClassLoader(new URL[] {location}, null); + } + + /** The value's own class identifies the owner in the simple case. */ + @Test + void testDefinedByMatchesTheDeclaringLoader() throws Exception { + try (URLClassLoader child = childLoaderOwning(Leaky.class)) { + Object leaky = child.loadClass(Leaky.class.getName()).getDeclaredConstructor().newInstance(); + assertTrue(ClassLoaderResourceCleanerUtils.definedBy(leaky, child)); + assertFalse(ClassLoaderResourceCleanerUtils.definedBy(leaky, Leaky.class.getClassLoader())); + } + } + + /** + * Caches such as Jackson's BufferRecycler park a SoftReference in a ThreadLocal. The reference is + * a bootstrap class, so only its referent identifies the owning catalog. + */ + @Test + void testDefinedByLooksThroughAReference() throws Exception { + try (URLClassLoader child = childLoaderOwning(Leaky.class)) { + Object leaky = child.loadClass(Leaky.class.getName()).getDeclaredConstructor().newInstance(); + assertTrue(ClassLoaderResourceCleanerUtils.definedBy(new SoftReference<>(leaky), child)); + assertTrue(ClassLoaderResourceCleanerUtils.definedBy(new WeakReference<>(leaky), child)); + } + } + + /** An empty reference names no owner and must not be mistaken for one. */ + @Test + void testDefinedByIgnoresNullAndClearedReferences() { + assertFalse(ClassLoaderResourceCleanerUtils.definedBy(null, getClass().getClassLoader())); + assertFalse( + ClassLoaderResourceCleanerUtils.definedBy( + new SoftReference<>(null), getClass().getClassLoader())); + } + + /** + * A thread local holding the catalog's object behind a SoftReference must be cleared. Left in + * place it keeps the catalog's ClassLoader alive until heap pressure clears the reference, which + * Metaspace pressure alone never triggers. + */ + @Test + void testClearThreadLocalMapClearsSoftReferencedValues() throws Exception { + try (URLClassLoader child = childLoaderOwning(Leaky.class)) { + Object leaky = child.loadClass(Leaky.class.getName()).getDeclaredConstructor().newInstance(); + SOFT_HOLDER.set(new SoftReference<>(leaky)); + + ClassLoaderResourceCleanerUtils.clearThreadLocalMap(Thread.currentThread(), child); + + assertNull(SOFT_HOLDER.get()); + } + } + + /** + * A driver's own housekeeping thread runs code the catalog defined, so the thread pins the loader + * whatever its context ClassLoader says. + */ + @Test + void testRunningWithClassLoaderMatchesTheRunnableOfAThread() throws Exception { + try (URLClassLoader child = childLoaderOwning(Leaky.class)) { + Runnable owned = + (Runnable) + child.loadClass(LeakyTask.class.getName()).getDeclaredConstructor().newInstance(); + Thread thread = new Thread(owned, "leaky-task"); + thread.setContextClassLoader(null); + + assertTrue(ClassLoaderResourceCleanerUtils.runningWithClassLoader(thread, child)); + assertFalse( + ClassLoaderResourceCleanerUtils.runningWithClassLoader( + new Thread(() -> {}, "unrelated"), child)); + } + } + + /** + * A thread that merely runs the catalog's code is not the catalog's to stop. A request thread + * serving an operation on the very catalog being dropped looks exactly like this, and + * interrupting it fails the request with "Thread was interrupted while waiting for lock". + */ + @Test + void testRunningWithClassLoaderIgnoresAThreadOnlyExecutingTheLoadersCode() throws Exception { + try (URLClassLoader child = childLoaderOwning(Leaky.class)) { + Runnable owned = + (Runnable) + child.loadClass(LeakyTask.class.getName()).getDeclaredConstructor().newInstance(); + // The worker owns neither side: its class and its runnable are the server's, and it just + // happens to be executing the catalog's code, which is how a pooled request thread looks. + Thread worker = new Thread(() -> owned.run(), "pooled-worker"); + worker.setDaemon(true); + worker.start(); + try { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (worker.getState() != Thread.State.TIMED_WAITING && System.nanoTime() < deadline) { + Thread.sleep(10); + } + assertEquals(Thread.State.TIMED_WAITING, worker.getState()); + + assertFalse(ClassLoaderResourceCleanerUtils.runningWithClassLoader(worker, child)); + } finally { + worker.interrupt(); + worker.join(TimeUnit.SECONDS.toMillis(5)); + } + } + } + + /** Providers installed by other loaders, and by the JDK itself, must be left alone. */ + @Test + void testRemoveSecurityProvidersLeavesUnrelatedProviders() throws Exception { + int before = Security.getProviders().length; + try (URLClassLoader child = childLoaderOwning(Leaky.class)) { + ClassLoaderResourceCleanerUtils.removeSecurityProviders(child); + } + assertEquals(before, Security.getProviders().length); + } + + /** Entries belonging to another loader must survive the sweep. */ + @Test + void testClearThreadLocalMapLeavesUnrelatedValues() throws Exception { + try (URLClassLoader child = childLoaderOwning(Leaky.class)) { + Object unrelated = new Object(); + UNRELATED_HOLDER.set(unrelated); + + ClassLoaderResourceCleanerUtils.clearThreadLocalMap(Thread.currentThread(), child); + + assertSame(unrelated, UNRELATED_HOLDER.get()); + } + } + /** * When a class is loaded by exactly the target classloader, isOwnedByClassLoader must return true * — the guard should allow static-state cleanup to proceed. diff --git a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientFactory.java b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientFactory.java index 54e8977601e..01c2c5b2285 100644 --- a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientFactory.java +++ b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientFactory.java @@ -33,6 +33,7 @@ import org.apache.gravitino.exceptions.GravitinoRuntimeException; import org.apache.gravitino.hive.kerberos.AuthenticationConfig; import org.apache.gravitino.hive.kerberos.HmsKerberosClient; +import org.apache.gravitino.utils.ClassLoaderResourceCleanerUtils; import org.apache.gravitino.utils.PrincipalUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.security.UserGroupInformation; @@ -260,6 +261,12 @@ public void close() { synchronized (classLoaderLock) { if (backendClassLoader != null) { + // The backend ClassLoader is a second, nested isolation layer that holds the catalog's + // own ClassLoader as its base. Closing it releases its jars but not the references other + // threads still hold to it: Hadoop's Shell runs sub-processes, and the JDK's pooled + // "process reaper" threads inherit the spawning thread's context ClassLoader, which is a + // GC root. Cleaning the nested loader clears those, so both layers become collectable. + ClassLoaderResourceCleanerUtils.closeClassLoaderResource(backendClassLoader); backendClassLoader.close(); backendClassLoader = null; } diff --git a/core/src/main/java/org/apache/gravitino/utils/ClassLoaderPool.java b/core/src/main/java/org/apache/gravitino/utils/ClassLoaderPool.java index 7ee2721a140..ccaaa4087c0 100644 --- a/core/src/main/java/org/apache/gravitino/utils/ClassLoaderPool.java +++ b/core/src/main/java/org/apache/gravitino/utils/ClassLoaderPool.java @@ -20,9 +20,6 @@ import java.io.Closeable; import java.net.URLClassLoader; -import java.sql.Driver; -import java.sql.DriverManager; -import java.util.Enumeration; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Supplier; @@ -182,8 +179,7 @@ private void removeAndCleanup(ClassLoaderKey key) { * Performs final cleanup when a ClassLoader's reference count reaches zero. This includes: * *
    - *
  1. Deregistering all JDBC drivers loaded by the ClassLoader - *
  2. Cleaning up ClassLoader resources (ThreadLocals, Hadoop FileSystem, etc.) + *
  3. Cleaning up ClassLoader resources (JDBC drivers, ThreadLocals, Hadoop FileSystem, etc.) *
  4. Closing the ClassLoader itself *
*/ @@ -198,9 +194,8 @@ private void doFinalCleanup(PooledClassLoaderEntry entry) { } /** - * Performs full resource cleanup for an {@link IsolatedClassLoader}: deregisters JDBC drivers, - * cleans up ClassLoader-scoped resources (ThreadLocals, Hadoop FileSystem, etc.), and closes the - * ClassLoader. + * Performs full resource cleanup for an {@link IsolatedClassLoader}: cleans up ClassLoader-scoped + * resources (JDBC drivers, ThreadLocals, Hadoop FileSystem, etc.) and closes the ClassLoader. * * @param classLoader The IsolatedClassLoader to clean up. */ @@ -208,7 +203,9 @@ public static void cleanupClassLoader(IsolatedClassLoader classLoader) { try { URLClassLoader internalCl = classLoader.getInternalClassLoader(); if (internalCl != null) { - deregisterAllDrivers(internalCl); + // closeClassLoaderResource deregisters the loader's JDBC drivers as well. It has to: from + // here DriverManager filters its drivers by the caller's class loader, so the catalog's + // drivers are neither visible nor removable outside their own loader. ClassLoaderResourceCleanerUtils.closeClassLoaderResource(internalCl); } } catch (Exception e) { @@ -216,26 +213,4 @@ public static void cleanupClassLoader(IsolatedClassLoader classLoader) { } classLoader.close(); } - - /** - * Deregisters all JDBC drivers that were loaded by the given ClassLoader. - * - * @param classLoader The ClassLoader whose drivers should be deregistered. - */ - private static void deregisterAllDrivers(ClassLoader classLoader) { - // DriverManager.getDrivers() returns a snapshot in JDK 9+, so iterating while - // calling deregisterDriver() is safe. - Enumeration drivers = DriverManager.getDrivers(); - while (drivers.hasMoreElements()) { - Driver driver = drivers.nextElement(); - if (driver.getClass().getClassLoader() == classLoader) { - try { - DriverManager.deregisterDriver(driver); - LOG.info("Deregistered JDBC driver {} for ClassLoader.", driver); - } catch (Exception e) { - LOG.warn("Failed to deregister JDBC driver {}", driver, e); - } - } - } - } }