This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new ea1d960a0e [#12986] fix(catalog): Release the ClassLoader of a dropped
catalog (#12987)
ea1d960a0e is described below
commit ea1d960a0edd086e19363a47596176970ed5c9d7
Author: Qi Yu <[email protected]>
AuthorDate: Wed Sep 9 14:10:41 2026 +0800
[#12986] fix(catalog): Release the ClassLoader of a dropped catalog (#12987)
### What changes were proposed in this pull request?
All in `ClassLoaderResourceCleanerUtils`, so every provider and every
caller of the cleaner benefits:
- `clearThreadLocalMap` looks **through** a `java.lang.ref.Reference`
value to its referent when deciding whether a thread-local entry belongs
to the dying loader, also checks the entry's key, and no longer skips
threads that are not named `Gravitino-webserver-*`.
- `runningWithClassLoader` matches a thread by the class of the thread
and of its runnable, not only by its context ClassLoader.
- New step `removeLoggerContextListeners`: removes listeners the loader
registered on the shared Log4j `LoggerContext`.
- New step `deregisterJdbcDrivers`: defines `JdbcDriverDeregisterer`
inside the catalog's loader and calls it there, because `DriverManager`
filters both `getDrivers()` and `deregisterDriver()` by the caller's
ClassLoader.
- New step `shutdownMysqlConnectionCleanup`: calls Connector/J's
`uncheckedShutdown()`.
- New step `removeSecurityProviders`: removes JCA providers the loader
installed.
- New step `clearResourceBundleCache`:
`ResourceBundle.clearCache(loader)`, since bundles are cached JVM-wide
behind soft references.
- `HiveClientFactory.close()` runs the cleaner against the nested
`HiveClientClassLoader` before closing it.
- `ClassLoaderPool.deregisterAllDrivers` is removed: it ran from the
server's ClassLoader, where the catalog's drivers are invisible, so it
never deregistered anything. The cleaner now covers it.
### Why are the changes needed?
Dropping or altering a catalog leaked its ClassLoader, so Metaspace grew
until the JVM could no longer load new classes and the server degraded
into per-feature 500s while already-warm paths kept returning 200. Since
an `alter` rebuilds the ClassLoader, the loss accumulated: five alters
of a Hive catalog cost ~48 MB that was never returned.
Every retention path was traced from a heap dump back to a GC root. They
are unrelated to each other, which is why the fix has several parts:
| pinned by | affected |
|---|---|
| commons-logging listener on the shared Log4j `LoggerContext` | hive |
| `SoftReference` in a `ThreadLocal` (Jackson `BufferRecycler`) |
paimon, iceberg, cloud filesets |
| nested `HiveClientClassLoader` never cleaned | hive |
| `DriverManager.registeredDrivers` | every JDBC catalog |
| PostgreSQL `LazyCleaner` thread | jdbc-postgresql |
| MySQL `AbandonedConnectionCleanupThread` executor | jdbc-mysql,
iceberg on a jdbc backend |
| JCA provider (`OpenSSLProvider` from the AWS bundle) | fileset on
abfss |
| `ResourceBundle` cache (soft) | Oracle's `ErrorMessages`, any
localized driver |
| a task on a shared executor (AWS SDK idle-connection reaper) | cloud
clients |
Soft references deserve a note: they are cleared under **heap**
pressure, and Metaspace pressure never triggers that, so on a server
with a roomy heap and a small `MaxMetaspaceSize` a soft-referenced
loader is permanent in practice.
Fix: #12986
### Does this PR introduce _any_ user-facing change?
No new configuration or API. Dropped catalogs release their Metaspace,
so a long-running server no longer grows without bound.
### How was this patch tested?
Unit tests: 7 new cases in `TestClassLoaderResourceCleanerUtils`
(looking through a reference, cleared references, clearing a
soft-referenced thread-local, leaving unrelated entries alone, matching
a thread by its runnable, leaving unrelated security providers alone).
`:catalogs:catalog-common`, `:catalogs:hive-metastore-common`,
`:catalogs:catalog-hive`, `:catalogs:catalog-fileset`,
`:catalogs:catalog-jdbc-common` and the `:core` ClassLoader tests pass.
End-to-end on a packaged server with `-Xms1024m -Xmx1024m
-XX:MaxMetaspaceSize=512m`, one catalog at a time: create, exercise
(schema plus a table, fileset, topic or model version), drop, force a
full GC, then count loaders with `jcmd VM.classloader_stats` and read
Metaspace with `jcmd GC.heap_info`. Backends were a real Hive metastore,
MySQL, PostgreSQL, Kafka and MinIO in containers.
| provider | classes loaded | loaders after drop, before | after |
|---|---|---|---|
| `model` | 9 | 0 | 0 |
| `fileset` (file://) | 803 | 0 | 0 |
| `fileset` (s3a, MinIO) | 3042 | retained | **0** |
| `fileset` (gs://) | 919 | retained | **0** |
| `fileset` (abfss://) | 1090 | retained | **0** |
| `jdbc-mysql` | 463 | retained | **0** |
| `jdbc-postgresql` | 311 | retained | **0** |
| `kafka` | 1072 | 0 | 0 |
| `hive` | 1263 (3 loaders) | retained | **0** |
| `lakehouse-paimon` | 1165 | retained | **0** |
| `lakehouse-iceberg` (jdbc backend) | 1433 | retained | **0** |
Repeated churn, the case that exhausts Metaspace in practice — five
alters of a Hive catalog: before, 7 loaders and 70.6 → 119.3 MB that
never came back; after, back to baseline at +0.8 MB.
`glue` is the one provider still not released, verified against
LocalStack. Its root is different in kind: the AWS SDK's
`IdleConnectionReaper` is a singleton per ClassLoader that only stops
once every connection manager is deregistered, and it ignores
interrupts, so some AWS client the catalog builds is not being closed.
That is a client-lifecycle bug in the catalog rather than a cleanup gap,
and papering over it by reflecting into SDK internals seemed worse than
reporting it; I will file it separately.
Not covered locally, for lack of a backend: `jdbc-doris`,
`jdbc-starrocks`, `lakehouse-hudi`, `lakehouse-generic`. Doris and
StarRocks use the MySQL driver, so the DriverManager and Connector/J
fixes apply to them unchanged.
```bash
./gradlew :catalogs:catalog-common:test
:catalogs:hive-metastore-common:test \
:catalogs:catalog-hive:test :catalogs:catalog-fileset:test \
:catalogs:catalog-jdbc-common:test -PskipITs -PskipWeb=true
```
https://claude.ai/code/session_013xVSteM2ZUjXRHFbHtayVK
---
.../utils/ClassLoaderResourceCleanerUtils.java | 229 ++++++++++++++++++++-
.../gravitino/utils/JdbcDriverDeregisterer.java | 65 ++++++
.../utils/TestClassLoaderResourceCleanerUtils.java | 154 ++++++++++++++
.../gravitino/hive/client/HiveClientFactory.java | 7 +
.../apache/gravitino/utils/ClassLoaderPool.java | 37 +---
5 files changed, 454 insertions(+), 38 deletions(-)
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 6a91f4ac48..b4c33ffa8a 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 class ClassLoaderResourceCleanerUtils {
// 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 @@ public class ClassLoaderResourceCleanerUtils {
}
}
- private static boolean runningWithClassLoader(Thread thread, ClassLoader
targetClassLoader) {
- return thread != null && thread.getContextClassLoader() ==
targetClassLoader;
+ /**
+ * Whether the thread belongs to the class loader being released.
+ *
+ * <p>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.
+ *
+ * <p>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 @@ public class ClassLoaderResourceCleanerUtils {
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 @@ public class ClassLoaderResourceCleanerUtils {
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 @@ public class ClassLoaderResourceCleanerUtils {
}
}
+ /**
+ * Whether {@code value}, or what it refers to when it is a {@link
Reference}, was defined by
+ * {@code classLoader}.
+ *
+ * <p>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 @@ public class ClassLoaderResourceCleanerUtils {
});
}
+ /**
+ * Removes shutdown listeners the class loader registered on the shared
Log4j {@code
+ * LoggerContext}.
+ *
+ * <p>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.
+ *
+ * <p>{@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.
+ *
+ * <p>{@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.
+ *
+ * <p>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}.
+ *
+ * <p>{@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 0000000000..824281b4da
--- /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}.
+ *
+ * <p>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<String> deregisterAll() {
+ ClassLoader owner = JdbcDriverDeregisterer.class.getClassLoader();
+ List<String> deregistered = new ArrayList<>();
+ Enumeration<Driver> 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 c5a241d8c7..52e4d10b6a 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<Object> SOFT_HOLDER = new ThreadLocal<>();
+ private static final ThreadLocal<Object> 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 54e8977601..01c2c5b228 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.commons.lang3.reflect.MethodUtils;
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 final class HiveClientFactory {
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 7ee2721a14..ccaaa4087c 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 @@ package org.apache.gravitino.utils;
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 @@ public class ClassLoaderPool implements Closeable {
* Performs final cleanup when a ClassLoader's reference count reaches zero.
This includes:
*
* <ol>
- * <li>Deregistering all JDBC drivers loaded by the ClassLoader
- * <li>Cleaning up ClassLoader resources (ThreadLocals, Hadoop FileSystem,
etc.)
+ * <li>Cleaning up ClassLoader resources (JDBC drivers, ThreadLocals,
Hadoop FileSystem, etc.)
* <li>Closing the ClassLoader itself
* </ol>
*/
@@ -198,9 +194,8 @@ public class ClassLoaderPool implements Closeable {
}
/**
- * 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 class ClassLoaderPool implements Closeable {
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 class ClassLoaderPool implements Closeable {
}
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<Driver> 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);
- }
- }
- }
- }
}
