OpenHFT / Java-Runtime-Compiler

Java Runtime Compiler
Other
639 stars 143 forks source link

Support JDKs with strong encapsulation #91

Open Marcono1234 opened 2 years ago

Marcono1234 commented 2 years ago

Recent JDK versions, such as JDK 17, strongly encapsulate their internals, see JEP 403. Therefore using your library fails for these versions by default because the internal JDK fields are not accessible:

java.lang.RuntimeException: java.lang.reflect.InaccessibleObjectException: Unable to make public java.lang.Iterable com.sun.tools.javac.file.JavacFileManager.listLocationsForModules(javax.tools.JavaFileManager$Location) throws java.io.IOException accessible: module jdk.compiler does not "exports com.sun.tools.javac.file" to unnamed module @c86b9e3
    at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.invocationHelper(JavacTaskImpl.java:168)
    at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:100)
    at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:94)
    at net.openhft.compiler.CachedCompiler.compileFromJava(CachedCompiler.java:112)
    at net.openhft.compiler.CachedCompiler.loadFromJava(CachedCompiler.java:151)
    <omitted>
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make public java.lang.Iterable com.sun.tools.javac.file.JavacFileManager.listLocationsForModules(javax.tools.JavaFileManager$Location) throws java.io.IOException accessible: module jdk.compiler does not "exports com.sun.tools.javac.file" to unnamed module @c86b9e3
    at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
    at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
    at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
    at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
    at net.openhft.compiler.MyJavaFileManager.invokeNamedMethodIfAvailable(MyJavaFileManager.java:214)
    at net.openhft.compiler.MyJavaFileManager.listLocationsForModules(MyJavaFileManager.java:77)
    at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedJavaFileManager.listLocationsForModules(ClientCodeWrapper.java:388)
    at jdk.compiler/com.sun.tools.javac.code.ModuleFinder$ModuleLocationIterator.hasNext(ModuleFinder.java:138)
    at jdk.compiler/com.sun.tools.javac.code.ModuleFinder.scanModulePath(ModuleFinder.java:294)
    at jdk.compiler/com.sun.tools.javac.code.ModuleFinder.findAllModules(ModuleFinder.java:187)
    at jdk.compiler/com.sun.tools.javac.comp.Modules.getUnnamedModuleCompleter(Modules.java:1434)
    at jdk.compiler/com.sun.tools.javac.comp.Modules.setCompilationUnitModules(Modules.java:471)
    at jdk.compiler/com.sun.tools.javac.comp.Modules.enter(Modules.java:265)
    at jdk.compiler/com.sun.tools.javac.comp.Modules.initModules(Modules.java:231)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.initModules(JavaCompiler.java:1021)
    at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:919)
    at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:104)
    at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.invocationHelper(JavacTaskImpl.java:152)
    ... 133 more

This can be worked around by using --add-opens jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED, but that is rather cumbersome and brittle. Would it be possible for this library to not rely on JDK internals (possibly at the cost that the user has to provide additional arguments)?

victornoel commented 2 years ago

I'm also getting the same with java.lang:

2022-05-11 10:25:07 | Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make protected final java.lang.Class java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int) throws java.lang.ClassFormatError accessible: module java.base does not "opens java.lang" to unnamed module @2b2948e2
2022-05-11 10:25:07 | at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
2022-05-11 10:25:07 | at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
2022-05-11 10:25:07 | at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
2022-05-11 10:25:07 | at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
2022-05-11 10:25:07 | at net.openhft.compiler.CompilerUtils.<clinit>(CompilerUtils.java:65)
2022-05-11 10:25:07 | ... 21 more
minborg commented 2 years ago

Here is a general article about running the Chronicle libraries under Java 17. Perhaps there are some findings there you can apply?

https://chronicle.software/chronicle-support-java-17/

victornoel commented 2 years ago

Thx @minborg, it helps.

In the end, for this module, I only had to add --add-opens java.base/java.lang=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED :)

Marcono1234 commented 2 years ago

@minborg, has this actually be fixed (because I don't see any related changes on develop or ea)? Using --add-opens is merely a workaround and cannot be applied easily in all usage scenarios. Additionally users of your library might be reluctant to use this because it risks that your library stops working for future Java versions, possibly even between patch versions of the JDK.

minborg commented 2 years ago

@minborg, has this actually be fixed (because I don't see any related changes on develop or ea)? Using --add-opens is merely a workaround and cannot be applied easily in all usage scenarios. Additionally users of your library might be reluctant to use this because it risks that your library stops working for future Java versions, possibly even between patch versions of the JDK.

This is a known issue and will be addressed in a later release.

victornoel commented 2 years ago

@minborg out of curiosity, what can be done from the lib point of view to improve this?

I've tried looking into this, and never really understood how this work except for authorizing those kind of accesses from the runtime JVM with --add-opens

minborg commented 2 years ago

Generally speaking, an obvious way to reduce problems like this is to remove the use of reflection (on objects that are not exported by the module system). Support for the module system itself is another way to improve library quality. That said, Java 8 does not support the module system.

victornoel commented 2 years ago

an obvious way to reduce problems like this is to remove the use of reflection

Indeed ^^

Support for the module system itself is another way to improve library quality

Ah that's what I am interested in. So it is possible for the lib to include some configuration that says it uses some closed APIs (such as the one discussed above) and it will work out of the box? I was originally worried that for security reason, it would always be the user of the lib that must decide what the lib can access :/

Marcono1234 commented 1 year ago

This is a known issue and will be addressed in a later release.

@minborg, would you mind reopening this issue? This problem still does not seem to be solved in the latest version, and I find it a bit weird / confusing to close an issue for a problem which is still unresolved (the solutions mentioned above are only workarounds).

I don't want to rush you in any way; I just think it would be more transparent to keep this issue open until the problem is solved. This will also make it easier for others to find this issue.