LuciferYang opened a new pull request, #7687: URL: https://github.com/apache/kyuubi/pull/7687
### 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 `ReflectUtils` uses throughout the engines, where the public-lookup fallback exists for exactly the case where the hidden lookup does not work. 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 its javadoc now states. Aggregating diagnostics there is left to a follow-up. 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 candidate carries it. `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 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
