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 132202914e [#13075] fix(catalog): inspect thread targets without
resolving subclass fields (#13076)
132202914e is described below
commit 132202914e94699b32a2b9b223030536c599f25c
Author: Qi Yu <[email protected]>
AuthorDate: Thu Sep 10 17:54:48 2026 +0800
[#13075] fix(catalog): inspect thread targets without resolving subclass
fields (#13076)
### What changes were proposed in this pull request?
Read the runnable from `Thread`'s declared `target` field, avoiding
subclass field resolution and shadowed fields. Catch `LinkageError`
during individual thread ownership checks so remaining cleanup can
continue, while allowing fatal VM errors to propagate.
Add regressions for missing subclass dependencies, shadowed targets,
linkage errors, and fatal-error propagation.
### Why are the changes needed?
Reflecting on an unrelated Hadoop thread can throw
`NoClassDefFoundError` during catalog cleanup and fail a DISABLE
request. A shadowed field can also cause the wrong thread ownership
decision.
Fix: #13075
### Does this PR introduce _any_ user-facing change?
Catalog cleanup no longer fails for these thread-inspection errors. No
public API or configuration changes.
### How was this patch tested?
- Before the fix, three regression tests failed, including the same
reflection/`NoClassDefFoundError` path seen in CI.
- main: `./gradlew spotlessApply :catalogs:catalog-common:check
-PskipITs -PskipDockerTests=true` — 35 tests passed.
- The same patch applied cleanly to branch-1.3; `./gradlew
:catalogs:catalog-common:check -PskipITs -PskipDockerTests=true` — 21
tests passed.
- `git diff --check` passed. Full Paimon integration tests were not
rerun.
Signed-off-by: yuqi <[email protected]>
---
.../utils/ClassLoaderResourceCleanerUtils.java | 19 ++--
.../utils/TestClassLoaderResourceCleanerUtils.java | 103 +++++++++++++++++++++
2 files changed, 115 insertions(+), 7 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 b4c33ffa8a..cff678a05c 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
@@ -197,17 +197,22 @@ public class ClassLoaderResourceCleanerUtils {
if (thread == null) {
return false;
}
- if (thread.getContextClassLoader() == targetClassLoader
- || thread.getClass().getClassLoader() == targetClassLoader) {
- return true;
- }
try {
- Object runnable = FieldUtils.readField(thread, "target", true);
+ if (thread.getContextClassLoader() == targetClassLoader
+ || thread.getClass().getClassLoader() == targetClassLoader) {
+ return true;
+ }
+ // Inspect Thread's own target, not a subclass's fields. Reflecting on a
driver thread
+ // subclass can resolve unavailable field types, or find a shadowed
target field.
+ Field targetField = Thread.class.getDeclaredField("target");
+ targetField.setAccessible(true);
+ Object runnable = targetField.get(thread);
if (runnable != null && runnable.getClass().getClassLoader() ==
targetClassLoader) {
return true;
}
- } catch (Exception e) {
- LOG.debug("Cannot read the runnable of thread {}", thread.getName(), e);
+ } catch (Exception | LinkageError e) {
+ // A stale thread's dependencies must not abort cleanup of the remaining
threads.
+ LOG.debug("Cannot inspect the classloader ownership of thread {}",
thread.getName(), e);
}
return false;
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 52e4d10b6a..953a75fdcf 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
@@ -23,6 +23,7 @@ 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.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.lang.ref.SoftReference;
@@ -53,6 +54,108 @@ class TestClassLoaderResourceCleanerUtils {
}
}
+ /** A thread whose unrelated field type can become unavailable during
classloader cleanup. */
+ public static class ThreadWithMissingDependency extends Thread {
+ /** A field whose type the isolated test loader deliberately cannot
resolve. */
+ public Leaky dependency;
+
+ /**
+ * Creates a thread with the supplied runnable.
+ *
+ * @param runnable the actual thread target
+ */
+ public ThreadWithMissingDependency(Runnable runnable) {
+ super(runnable);
+ }
+ }
+
+ private static class ThreadWithShadowedTarget extends Thread {
+ @SuppressWarnings("unused")
+ private final Runnable target;
+
+ private ThreadWithShadowedTarget(Runnable actualTarget, Runnable
shadowedTarget) {
+ super(actualTarget);
+ target = shadowedTarget;
+ setContextClassLoader(null);
+ }
+ }
+
+ @Test
+ void testThreadInspectionDoesNotResolveSubclassFieldTypes() throws Exception
{
+ URL location =
getClass().getProtectionDomain().getCodeSource().getLocation();
+ try (URLClassLoader child =
+ new URLClassLoader(new URL[] {location}, null) {
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws
ClassNotFoundException {
+ if (name.equals(Leaky.class.getName())) {
+ throw new ClassNotFoundException(name);
+ }
+ return super.loadClass(name, resolve);
+ }
+ }) {
+ Thread thread =
+ (Thread)
+ child
+ .loadClass(ThreadWithMissingDependency.class.getName())
+ .getConstructor(Runnable.class)
+ .newInstance((Runnable) () -> {});
+ thread.setContextClassLoader(null);
+ assertThrows(NoClassDefFoundError.class, () ->
thread.getClass().getDeclaredFields());
+ assertTrue(
+ ClassLoaderResourceCleanerUtils.runningWithClassLoader(
+ thread, getClass().getClassLoader()));
+ }
+ }
+
+ @Test
+ void testThreadInspectionIgnoresShadowedTarget() throws Exception {
+ try (URLClassLoader child = childLoaderOwning(LeakyTask.class)) {
+ Runnable owned =
+ (Runnable)
+
child.loadClass(LeakyTask.class.getName()).getDeclaredConstructor().newInstance();
+ Runnable unrelated = () -> {};
+ assertTrue(
+ ClassLoaderResourceCleanerUtils.runningWithClassLoader(
+ new ThreadWithShadowedTarget(owned, unrelated), child));
+ assertFalse(
+ ClassLoaderResourceCleanerUtils.runningWithClassLoader(
+ new ThreadWithShadowedTarget(unrelated, owned), child));
+ }
+ }
+
+ @Test
+ void testThreadInspectionToleratesLinkageErrors() {
+ Thread thread =
+ new Thread() {
+ @Override
+ public ClassLoader getContextClassLoader() {
+ throw new NoClassDefFoundError("unavailable driver dependency");
+ }
+ };
+ assertFalse(
+ ClassLoaderResourceCleanerUtils.runningWithClassLoader(
+ thread, getClass().getClassLoader()));
+ }
+
+ @Test
+ void testThreadInspectionDoesNotSwallowFatalErrors() {
+ OutOfMemoryError failure = new OutOfMemoryError("test error");
+ Thread thread =
+ new Thread() {
+ @Override
+ public ClassLoader getContextClassLoader() {
+ throw failure;
+ }
+ };
+ assertSame(
+ failure,
+ assertThrows(
+ OutOfMemoryError.class,
+ () ->
+ ClassLoaderResourceCleanerUtils.runningWithClassLoader(
+ thread, getClass().getClassLoader())));
+ }
+
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.
