Java-Runtime-Compiler icon indicating copy to clipboard operation
Java-Runtime-Compiler copied to clipboard

Support JDKs with strong encapsulation

Open Marcono1234 opened this issue 3 years ago • 9 comments

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)?

Marcono1234 avatar Jan 18 '22 00:01 Marcono1234

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

victornoel avatar May 11 '22 08:05 victornoel

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/

minborg avatar May 11 '22 09:05 minborg

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 :)

victornoel avatar May 13 '22 16:05 victornoel

@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.

Marcono1234 avatar May 16 '22 18:05 Marcono1234

@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.

minborg avatar May 17 '22 07:05 minborg

@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

victornoel avatar May 17 '22 07:05 victornoel

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.

minborg avatar May 17 '22 07:05 minborg

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 :/

victornoel avatar May 17 '22 08:05 victornoel

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.

Marcono1234 avatar Oct 17 '22 21:10 Marcono1234