Reflection on internal classes fails under Java 9

Description

Due to changes in reflective access for the Jigsaw module system in Java 9, the Reflector will now fail on some cases that worked in previous Java versions.

Here fac will be an instance of com.sun.xml.internal.stream.XMLInputFactoryImpl, which is an extension of javax.xml.stream.XMLInputFactory. In the new java.xml module, javax.xml.stream is an exported package, but the XMLInputFactoryImpl is an internal extension of the public abstract class in that package. The invocation of createXMLStreamReader will be reflective and the Reflector will attempt to invoke the method based on the implementation class, which is not accessible outside the module, yielding:

A workaround here is to avoid the reflective call by type-hinting to the public exported interface but that defeats the point of reflection support:

Another (undesirable) workaround is to export the private package from java.xml to the unnamed module (which is the module used when code is loaded from the classpath rather than from a module) when invoking java/javac:

Alternatives:

We have investigated a number of options to enhance the Reflector. When we are trying to reflect, the only class we know is the class of the target object (here a non-public class, but one we can look at due to the default settings of --illegal-access). Some options considered include:

  • checking canAccess() on the Method (here it returns true, so no help)

  • invoking and looking for IllegalAccessError (pretty gross)

  • traversing the super class and interface hierarchy to find "more public" instances (this really is replicating logic that already exists in the Java method lookup logic)

None of these is "good".

Proposed:

This continues to be a warning on Java 9, 10, and (just released) 11. The warning is accurate - you are using reflection to a method on a non-exported class. The default setting in Java 9, 10, 11 is --illegal-access=permit which allows this but reports a warning on the first occurrence (other settings are warn, debug, and deny). In some future Java, the default may be deny.

If at some point in the future the JVM changes the default to deny, we still want these reflective calls to work. In that case, we will know this situation has occurred because Method#canAccess will return false. When that happens, we need to look up through the superclasses of the target object to find one that has an accessible version of the method.

Patch: clj-2066-6.patch - when invoking an instance method, map each method to an accessible super class method, then filter non-null methods found (presumably all of them). When on Java 8 or when --illegal-access=permit (the default on Java 9, 10, and 11), then the canAccess() check on each method will return true, allowing it to occur. If --illegal-access=deny, then canAccess() will be false and super class methods will be used instead.

Cases to test for screening (all with example at top):

  • Java 8 - should work with no warnings

  • Java 9, 10, and 11 with default JVM options (should succeed, but warn per --illegal-access=permit)

  • Java 9, 10, and 11 with --illegal-access=deny (should succeed, with no warnings)

  • Java 9, 10, and 11 with "--add-opens java.xml/com.sun.xml.internal.stream=ALL-UNNAMED" - should succeed, with no warnings

More info:

Environment

None

Activity

Show:
tcrawley
November 4, 2017, 1:38 AM

I realized while responding to the mailing-list thread for beta4 that this patch actually does nothing under non-EA builds of Java 9, since it no longer throws on illegal reflective access, it just warns. I believe the only way to avoid the warning (without code compiled for Java 9) would be to calculate the ancestor chain, then start from the top of that, asking for the method. However, we'd have to do that for every reflective call, since we'd have no way to know when we have to do it to avoid the warning.

Ghadi Shayban
August 24, 2018, 8:29 PM

Since illegal reflective ops are going to be denied in the near future, this is still a relevant issue. You can simulate the pending future behavior by adding the switch --illegal-access=deny on JDK9+

Now that Clojure is JDK1.8 minimum, a new way around the IllegalAccess catching is available to us, and it doesn't involve multirelease jars or anything messy like that.

The predicate j.l.reflect.Method::canAccess(targetObj) allows you to determine accessibility, and it respects modules. It will return false if you are calling a non-exported Method (e.g. on the XML impl subclass), but true on the public method upstairs in the chain. pseudocode:

This will help avoid the catching and rethrowing. We'll still need to have the ancestor lookup logic as in the current patch.

Ghadi Shayban
August 26, 2018, 3:18 AM

Adding a patch with the approach I suggested above. Relevant trace output of the motiving repro case.

Java 10 Denying Access -- Future default setting

Java 10 Default Setting

Java 8

%

Alex Miller
October 2, 2018, 10:28 PM

Apply the patch and {{mvn clean test }} (with Java 8, since that's our build setup).

To test, I am using a deps.edn in my clojure repo like this:

You can then test in the various modes (swapping JDK independently) with:

Alex Miller
October 3, 2018, 8:09 PM

Small tweaks in -6 patch

Completed

Assignee

Unassigned

Reporter

tcrawley

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Priority

Critical
Configure