You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by ch...@apache.org on 2026/09/02 10:18:59 UTC
(kyuubi) branch master updated: [KYUUBI #7686][UTIL] Treat strongly-encapsulated members as candidate misses in hiddenImpl
This is an automated email from the ASF dual-hosted git repository.
pan3793 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new 6ac41a4c7e [KYUUBI #7686][UTIL] Treat strongly-encapsulated members as candidate misses in hiddenImpl
6ac41a4c7e is described below
commit 6ac41a4c7ea281c1a2a8bbb26b64a82a829ca9dc
Author: YangJie <ya...@baidu.com>
AuthorDate: Wed Sep 2 18:18:50 2026 +0800
[KYUUBI #7686][UTIL] Treat strongly-encapsulated members as candidate misses in hiddenImpl
### Why are the changes needed?
Closes #7686.
`setAccessible` on a member of a package that is not open throws `java.lang.reflect.InaccessibleObjectException`. That happens since JDK 9 for named modules, and by default from the unnamed module since JDK 16. It is a `RuntimeException` that none of the three `hiddenImpl` builders caught, so one inaccessible candidate escaped the builder and aborted the whole lookup chain instead of falling through to the next implementation. That breaks the `hiddenImpl(...).impl(...)` pattern `Refle [...]
The exception type does not exist on Java 8, which this module compiles against, so the catch matches it by class name and rethrows anything else unchanged.
`DynFields` also records the exception with the candidate, so the `module ... does not "opens ..."` text that names the package to open survives into the `Cannot find field from candidates: ...` failure. Single-candidate call sites have no fallback chain to save and would otherwise be left with a bare cannot-find error that reads like a missing field; `ExecutePython`'s `ProcessImpl.pid` lookup is one of them. `DynMethods.Builder` has no per-candidate structure to record into, which it [...]
Neither upstream copy catches this exception either. parquet-common and iceberg-common both catch only `SecurityException` plus the `NoSuchMethod`/`NoSuchField` variant, on the releases these files were copied from and on current master, so this is a divergence Kyuubi carries deliberately and each `hiddenImpl` javadoc records it.
### How was this patch tested?
New tests in all three test classes, each branching on the running JDK because the behavior is version-dependent.
`testHiddenImplHandlesStronglyEncapsulatedMethods`, `testHiddenImplTreatsStronglyEncapsulatedFieldsAsMisses` and `testHiddenImplHandlesStronglyEncapsulatedConstructors`: on JDK 16+ the hidden lookup of a `java.math` member must fail as a candidate miss instead of letting the exception escape; on JDK 8 through 15 the same lookup still succeeds. `DynConstructorsTest` also asserts the `InaccessibleObjectException` is among the suppressed problems, and `DynFieldsTest` asserts the recorded [...]
`testHiddenImplPropagatesUnrelatedFailures` in all three: a `RuntimeException` that is not `InaccessibleObjectException` still escapes the builder. These pass before and after the change. They are here to stop the new catch from being widened into a catch-all, not to cover the fix.
`java.math` is the probe package because the surefire JVM opens `java.lang` but leaves `java.math` closed.
Ran on Zulu 17.0.18: `build/mvn -o test -pl kyuubi-util -am -DwildcardSuites=none` gives 39/39 green, and `build/mvn -o spotless:check -pl kyuubi-util` is clean. Reverting the `DynFields` candidate recording turns the `InaccessibleObjectException` assertion in `DynFieldsTest` red, so that assertion fails if the recording goes away. The JDK 8 leg of these tests runs in CI's `scala-test` job.
### Was this patch assisted by generative AI tooling?
Assisted-by: Claude Opus 5
Closes #7687 from LuciferYang/kyuubi-util-reflect-hiddenimpl-inaccessible.
Closes #7686
b6914f152 [yangjie01] [KYUUBI #7686][UTIL] Treat strongly-encapsulated members as candidate misses in hiddenImpl
Authored-by: YangJie <ya...@baidu.com>
Signed-off-by: Cheng Pan <ch...@apache.org>
---
.../kyuubi/util/reflect/DynConstructors.java | 26 ++++++++++++
.../org/apache/kyuubi/util/reflect/DynFields.java | 22 ++++++++++
.../org/apache/kyuubi/util/reflect/DynMethods.java | 18 ++++++++
.../kyuubi/util/reflect/DynConstructorsTest.java | 48 ++++++++++++++++++++++
.../apache/kyuubi/util/reflect/DynFieldsTest.java | 47 +++++++++++++++++++++
.../apache/kyuubi/util/reflect/DynMethodsTest.java | 47 +++++++++++++++++++++
6 files changed, 208 insertions(+)
diff --git a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynConstructors.java b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynConstructors.java
index 71445bc2c4..fd4ec71262 100644
--- a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynConstructors.java
+++ b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynConstructors.java
@@ -32,6 +32,11 @@ public class DynConstructors {
private DynConstructors() {}
+ // The type is absent from Java 8, which this module compiles against, so it is matched
+ // by name where it is caught.
+ private static final String INACCESSIBLE_OBJECT_EXCEPTION =
+ "java.lang.reflect.InaccessibleObjectException";
+
public static class Ctor<C> extends DynMethods.UnboundMethod {
private final Constructor<C> ctor;
private final Class<? extends C> constructed;
@@ -211,6 +216,18 @@ public class DynConstructors {
return this;
}
+ /**
+ * Checks for a hidden implementation.
+ *
+ * <p>Neither upstream copy catches InaccessibleObjectException from {@code setAccessible}, so
+ * under strong encapsulation one inaccessible constructor aborts the whole fallback chain,
+ * while this copy counts it as a miss like the other lookup failures.
+ *
+ * @param targetClass a class instance
+ * @param types argument classes for the constructor
+ * @return this Builder for method chaining
+ * @see Class#getDeclaredConstructor(Class[])
+ */
public <T> Builder hiddenImpl(Class<T> targetClass, Class<?>... types) {
// don't do any work if an implementation has been found
if (ctor != null) {
@@ -227,6 +244,15 @@ public class DynConstructors {
} catch (NoSuchMethodException e) {
// not the right implementation
problems.put(methodName(targetClass, types), e);
+ } catch (RuntimeException e) {
+ // setAccessible on a member of a package that is not open reports
+ // InaccessibleObjectException: since JDK 9 for named modules, since JDK 16 by
+ // default from the unnamed module; count it as a candidate miss like the
+ // failures above instead of letting it escape the fallback chain.
+ if (!INACCESSIBLE_OBJECT_EXCEPTION.equals(e.getClass().getName())) {
+ throw e;
+ }
+ problems.put(methodName(targetClass, types), e);
}
return this;
}
diff --git a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java
index 51fb72e556..ee930b21ff 100644
--- a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java
+++ b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynFields.java
@@ -31,6 +31,11 @@ public class DynFields {
private DynFields() {}
+ // The type is absent from Java 8, which this module compiles against, so it is matched
+ // by name where it is caught.
+ private static final String INACCESSIBLE_OBJECT_EXCEPTION =
+ "java.lang.reflect.InaccessibleObjectException";
+
/**
* Convenience wrapper class around {@link Field}.
*
@@ -311,6 +316,12 @@ public class DynFields {
/**
* Checks for a hidden implementation.
*
+ * <p>Upstream iceberg-common, which this class was copied from, does not catch
+ * InaccessibleObjectException from {@code setAccessible}, so under strong encapsulation one
+ * inaccessible field aborts the whole fallback chain. This copy counts it as a miss like the
+ * other lookup failures, and records the exception with the candidate so its "does not opens"
+ * text survives into the build failure.
+ *
* @param targetClass a class instance
* @param fieldName name of a field
* @return this Builder for method chaining
@@ -331,6 +342,17 @@ public class DynFields {
} catch (SecurityException | NoSuchFieldException e) {
// unusable
candidates.add(targetClass.getName() + "." + fieldName);
+ } catch (RuntimeException e) {
+ // setAccessible on a member of a package that is not open reports
+ // InaccessibleObjectException: since JDK 9 for named modules, since JDK 16 by
+ // default from the unnamed module; count it as a candidate miss like the
+ // failures above instead of letting it escape the fallback chain.
+ if (!INACCESSIBLE_OBJECT_EXCEPTION.equals(e.getClass().getName())) {
+ throw e;
+ }
+ // Keep the reason: the message names the module and package to open, and a
+ // single-candidate builder has no other way to surface it.
+ candidates.add(targetClass.getName() + "." + fieldName + " [" + e + "]");
}
return this;
}
diff --git a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynMethods.java b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynMethods.java
index 0a5a2b038f..5b86873a6a 100644
--- a/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynMethods.java
+++ b/kyuubi-util/src/main/java/org/apache/kyuubi/util/reflect/DynMethods.java
@@ -31,6 +31,11 @@ public class DynMethods {
private DynMethods() {}
+ // The type is absent from Java 8, which this module compiles against, so it is matched
+ // by name where it is caught.
+ private static final String INACCESSIBLE_OBJECT_EXCEPTION =
+ "java.lang.reflect.InaccessibleObjectException";
+
/**
* Convenience wrapper class around {@link Method}.
*
@@ -396,6 +401,11 @@ public class DynMethods {
/**
* Checks for a method implementation.
*
+ * <p>Neither upstream copy catches InaccessibleObjectException from {@code setAccessible}, so
+ * under strong encapsulation one inaccessible method aborts the whole fallback chain. This copy
+ * skips it like the other lookup failures instead of letting it escape. The builder keeps no
+ * per-candidate detail, so the exception's "does not opens" text is discarded with it.
+ *
* @param targetClass a class instance
* @param methodName name of a method (different from constructor)
* @param argClasses argument classes for the method
@@ -415,6 +425,14 @@ public class DynMethods {
this.method = new UnboundMethod(hidden, name);
} catch (SecurityException | NoSuchMethodException e) {
// unusable or not the right implementation
+ } catch (RuntimeException e) {
+ // setAccessible on a member of a package that is not open reports
+ // InaccessibleObjectException: since JDK 9 for named modules, since JDK 16 by
+ // default from the unnamed module; discard it as a candidate miss like the
+ // failures above instead of letting it escape the fallback chain.
+ if (!INACCESSIBLE_OBJECT_EXCEPTION.equals(e.getClass().getName())) {
+ throw e;
+ }
}
return this;
}
diff --git a/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynConstructorsTest.java b/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynConstructorsTest.java
index 3435fa889f..dc814ecd15 100644
--- a/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynConstructorsTest.java
+++ b/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynConstructorsTest.java
@@ -23,10 +23,17 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.math.BigInteger;
+import java.util.Arrays;
import org.junit.jupiter.api.Test;
public class DynConstructorsTest {
+ // "1.8" -> 8, "9" -> 9, "17" -> 17, etc.
+ private static final int JAVA_SPEC_VER =
+ Math.max(
+ 8, Integer.parseInt(System.getProperty("java.specification.version").split("\\.")[0]));
+
// Prefix of the IllegalArgumentException that Constructor.newInstance throws when the argument
// count does not fit the constructor. Matched as a prefix because the JDK 18+ accessor appends
// ": 3 expected: 2"; the legacy accessor stops at the prefix, and this build requests it with
@@ -127,4 +134,45 @@ public class DynConstructorsTest {
assertEquals("value", fixedArity.value);
}
+
+ @Test
+ public void testHiddenImplHandlesStronglyEncapsulatedConstructors() throws Exception {
+ // The test JVM opens java.base/java.lang to the unnamed module, so the encapsulated
+ // path needs a package the surefire configuration leaves closed; java.math is one.
+ if (JAVA_SPEC_VER >= 16) {
+ // setAccessible on the private BigInteger(int[]) constructor reports
+ // java.lang.reflect.InaccessibleObjectException; the builder must count it as a
+ // candidate miss instead of letting it escape the fallback chain.
+ NoSuchMethodException thrown =
+ assertThrows(
+ NoSuchMethodException.class,
+ () ->
+ DynConstructors.builder()
+ .hiddenImpl("java.math.BigInteger", int[].class)
+ .buildChecked());
+ assertTrue(thrown.getMessage().contains("Cannot find constructor"));
+ // a plain NoSuchMethodException would take the same branch if the JDK ever drops
+ // this constructor; the suppressed problem pins the InaccessibleObjectException path
+ assertTrue(
+ Arrays.stream(thrown.getSuppressed())
+ .anyMatch(
+ problem ->
+ "java.lang.reflect.InaccessibleObjectException"
+ .equals(problem.getClass().getName())));
+ } else {
+ // before strong encapsulation the same hidden lookup succeeds
+ DynConstructors.Ctor<?> ctor =
+ DynConstructors.builder().hiddenImpl("java.math.BigInteger", int[].class).buildChecked();
+ assertEquals(BigInteger.class, ctor.getConstructedClass());
+ }
+ }
+
+ @Test
+ public void testHiddenImplPropagatesUnrelatedFailures() {
+ // a RuntimeException that is not InaccessibleObjectException must still escape the
+ // builder instead of being counted as a candidate miss
+ // a builder with no class set dereferences the null base class inside the try block
+ assertThrows(
+ NullPointerException.class, () -> DynConstructors.builder().hiddenImpl((Class<?>[]) null));
+ }
}
diff --git a/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynFieldsTest.java b/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynFieldsTest.java
index ecf8ea2567..daad6202bb 100644
--- a/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynFieldsTest.java
+++ b/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynFieldsTest.java
@@ -21,14 +21,21 @@ package org.apache.kyuubi.util.reflect;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.math.BigInteger;
import org.junit.jupiter.api.Test;
public class DynFieldsTest {
+ // "1.8" -> 8, "9" -> 9, "17" -> 17, etc.
+ private static final int JAVA_SPEC_VER =
+ Math.max(
+ 8, Integer.parseInt(System.getProperty("java.specification.version").split("\\.")[0]));
+
private static class ReflectionTarget {
public static String staticField = "static-value";
@@ -166,4 +173,44 @@ public class DynFieldsTest {
assertEquals("new-value", bound.get());
assertEquals("new-value", target.instanceField);
}
+
+ @Test
+ public void testHiddenImplTreatsStronglyEncapsulatedFieldsAsMisses() throws Exception {
+ // Pin the probe: if a future JDK drops this member the lookup would miss for an unrelated
+ // reason and the assertions below would pass while covering nothing.
+ assertNotNull(BigInteger.class.getDeclaredField("signum"));
+
+ // The test JVM opens java.base/java.lang to the unnamed module, so the encapsulated
+ // path needs a package the surefire configuration leaves closed; java.math is one.
+ if (JAVA_SPEC_VER >= 16) {
+ // setAccessible on BigInteger.signum reports InaccessibleObjectException; the builder
+ // must count it as a candidate miss instead of letting it escape the fallback chain.
+ RuntimeException thrown =
+ assertThrows(
+ RuntimeException.class,
+ () -> DynFields.builder().hiddenImpl("java.math.BigInteger", "signum").build());
+ assertEquals(RuntimeException.class, thrown.getClass());
+ assertTrue(thrown.getMessage().contains("Cannot find field"));
+ // the candidate carries the exception, so these pin the encapsulation path itself rather
+ // than any candidate miss, and keep the module and package to open in the message
+ assertTrue(thrown.getMessage().contains("InaccessibleObjectException"));
+ assertTrue(thrown.getMessage().contains("opens java.math"));
+ } else {
+ // before strong encapsulation the same hidden lookup succeeds
+ DynFields.UnboundField<?> signum =
+ DynFields.builder().hiddenImpl("java.math.BigInteger", "signum").build();
+ assertNotNull(signum.bind(BigInteger.ONE).get());
+ }
+ }
+
+ @Test
+ public void testHiddenImplPropagatesUnrelatedFailures() {
+ // a RuntimeException that is not InaccessibleObjectException must still escape the
+ // builder instead of being counted as a candidate miss. The null goes to fieldName
+ // because hiddenImpl guards targetClass == null at the top; getDeclaredField(null) NPEs
+ // on every supported JDK (name.intern() on 8/11, requireNonNull at entry on 17+).
+ assertThrows(
+ NullPointerException.class,
+ () -> DynFields.builder().hiddenImpl(ReflectionTarget.class, null));
+ }
}
diff --git a/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynMethodsTest.java b/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynMethodsTest.java
index 914740c45b..1734c47cce 100644
--- a/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynMethodsTest.java
+++ b/kyuubi-util/src/test/java/org/apache/kyuubi/util/reflect/DynMethodsTest.java
@@ -18,15 +18,22 @@
package org.apache.kyuubi.util.reflect;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
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.io.IOException;
+import java.math.BigDecimal;
import org.junit.jupiter.api.Test;
public class DynMethodsTest {
+ // "1.8" -> 8, "9" -> 9, "17" -> 17, etc.
+ private static final int JAVA_SPEC_VER =
+ Math.max(
+ 8, Integer.parseInt(System.getProperty("java.specification.version").split("\\.")[0]));
+
public static class ReflectionTarget {
final IllegalStateException causelessFailure = new IllegalStateException();
@@ -199,4 +206,44 @@ public class DynMethodsTest {
// why invokeChecked skips the copy only at equal arity.
assertEquals("a/null", join.invoke(new ReflectionTarget(), "a"));
}
+
+ @Test
+ public void testHiddenImplHandlesStronglyEncapsulatedMethods() throws Exception {
+ // Pin the probe: if a future JDK drops this member the lookup would miss for an unrelated
+ // reason and the assertions below would pass while covering nothing.
+ assertNotNull(BigDecimal.class.getDeclaredMethod("checkScale", long.class));
+
+ // The test JVM opens java.base/java.lang to the unnamed module, so the encapsulated
+ // path needs a package the surefire configuration leaves closed; java.math is one.
+ if (JAVA_SPEC_VER >= 16) {
+ // setAccessible on BigDecimal.checkScale reports InaccessibleObjectException; the
+ // builder must count it as a candidate miss instead of letting it escape the fallback
+ // chain.
+ RuntimeException thrown =
+ assertThrows(
+ RuntimeException.class,
+ () ->
+ DynMethods.builder("checkScale")
+ .hiddenImpl("java.math.BigDecimal", long.class)
+ .build());
+ assertEquals(RuntimeException.class, thrown.getClass());
+ assertTrue(thrown.getMessage().contains("Cannot find method"));
+ // this builder records no detail for a miss, so a plain NoSuchMethodException from a
+ // removed method would take the same branch; the IOE path itself is not
+ // distinguishable here the way DynFields and DynConstructors distinguish it
+ } else {
+ // before strong encapsulation the same hidden lookup succeeds
+ assertNotNull(
+ DynMethods.builder("checkScale").hiddenImpl("java.math.BigDecimal", long.class).build());
+ }
+ }
+
+ @Test
+ public void testHiddenImplPropagatesUnrelatedFailures() {
+ // a RuntimeException that is not InaccessibleObjectException must still escape the
+ // builder instead of being counted as a candidate miss
+ assertThrows(
+ NullPointerException.class,
+ () -> DynMethods.builder("checkScale").hiddenImpl((Class<?>) null, "checkScale"));
+ }
}