Reflection on internal classes fails under Java 9
Description
Environment
Attachments
Activity
Alex Miller October 3, 2018 at 8:09 PM
Small tweaks in -6 patch
Alex Miller October 2, 2018 at 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:
Ghadi Shayban August 26, 2018 at 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
%
Ghadi Shayban August 24, 2018 at 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.
tcrawley November 4, 2017 at 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.
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 ofcom.sun.xml.internal.stream.XMLInputFactoryImpl
, which is an extension ofjavax.xml.stream.XMLInputFactory
. In the newjava.xml
module,javax.xml.stream
is an exported package, but theXMLInputFactoryImpl
is an internal extension of the public abstract class in that package. The invocation ofcreateXMLStreamReader
will be reflective and theReflector
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:
For original discussion, see https://groups.google.com/d/msg/clojure-dev/Tp_WuEEcdWI/LMMQVAUYBwAJ (note this from Java 9 EA which had different behavior at that time)
For reference info, see http://openjdk.java.net/projects/jigsaw/spec/sotms/#reflective-readability (also revised later)
Jigsaw discussion: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-May/012673.html
Relaxed strong encapsulation - adding illegal-access support: http://openjdk.java.net/jeps/261#Relaxed-strong-encapsulation
JLS 7.7: https://docs.oracle.com/javase/specs/jls/se9/html/jls-6.html#jls-7.7 (see notes on reflective module access)