JDK11 reporting illegal reflective access about parboiled HOT 25 CLOSED

Just released parboiled 1.4.0 which should fix the problem on Java 17.

from parboiled.

WARNING: Please consider reporting this to the maintainers of org.parboiled.transform.AsmUtils
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

it is very possible, that Java 12 will halt with this lib on classpath

from parboiled.

with OpenJDK 17 (next Long Term Support release), this will become more fun - https://openjdk.java.net/jeps/403:

It will no longer be possible to relax the strong encapsulation of internal elements via a single command-line option, as was possible in JDK 9 through JDK 16.

from parboiled.

Not sure if it is related, but I'm hitting a similar warning on 1.3.1 (Java 9)

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.parboiled.transform.AsmUtils (file:/C:/Users/Mike/.m2/repository/org/parboiled/parboiled-java/1.3.1/parboiled-java-1.3.1.jar) to method java.lang.ClassLoader.findLoadedClass(java.lang.String)
WARNING: Please consider reporting this to the maintainers of org.parboiled.transform.AsmUtils
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

from parboiled.

AsmUtils.loadClass utilizes defineClass which doesn't have JDK agnostic alternative so bundling the bytebuddy that has ClassInjector for this purpose would be a viable option. Another approach would be to indirectly provide the class injector as function. AsmUtils.findLoadedClass is even worse since to my knowledge there is no counterpart for that in JDK11. Luckily the already processed classes can be tracked in a class injector function.

ASMSettings also dictates what JDK level can be used in Vars and Actions. I got constant pool errors with static interface methods since the version isn't derived from the given parser class but instead hardcoded to JDK7. ClassFileVersion of bytebuddy has this kind of feature so it really would be a smart addition to the dependencies.

This is how my ParserTransformer.transformParser looks when bytebuddy is applied via function. The actual workaround is in form of a gitst patch, but this is how I bypass the warning and more-over get my > JDK7 parser implementation to work.

https://gist.github.com/TuomasKiviaho/3d6dae166e85c4aaba94afb0318b4e4d

PS. I'm personally not using Parboiled.createParser since the parser instance isn't thread-safe. Hence the example only contains how to generate a working parser in whichever environment the bytebuddy is capable of supporting.

    public static <T> Class<? extends T> transformParser(Class<T> parserClass)
        throws Exception
    {
        ClassInjector classInjector = (ClassInjector.UsingLookup.isAvailable()
            ? Optional.of(parserClass).<ClassInjector> map(
                ClassInjector.UsingLookup.of(MethodHandles.lookup())::in)
            : Optional.<ClassInjector> empty()).orElseGet(() -> {
                ClassLoader classLoader = parserClass.getClassLoader();
                ProtectionDomain protectionDomain = parserClass.getProtectionDomain();
                return ClassInjector.UsingUnsafe.isAvailable()
                    ? new ClassInjector.UsingUnsafe(classLoader, protectionDomain)
                    : new ClassInjector.UsingReflection(classLoader, protectionDomain);
            });
        Map<String, Class<?>> groupClasses = new TreeMap<>();
        return ParserTransformer.transformParser(parserClass,
            (className, groupClassGenerator) -> {
                return groupClasses.computeIfAbsent(className, key -> {
                    byte[] groupClassCode = groupClassGenerator.get();
                    Map<String, Class<?>> groupClass = classInjector.injectRaw(
                        Collections.singletonMap(className, groupClassCode));
                    return groupClass.get(className);
                });
            }, () -> {
                try
                {
                    return ClassFileVersion.of(parserClass).getMinorMajorVersion();
                }
                catch (IOException e)
                {
                    throw new UncheckedIOException(e);
                }
            });
    }

from parboiled.

Is there any fix for this? Is there a version of parboiled that doesn't trigger warnings on Java 11 and also supports Java 8?

from parboiled.

another example

from parboiled.

Hello all also facing this issue (java 11), is there any intent to fix it ?

from parboiled.

The intent is there but there's no capacity on my side for looking into this.
However, I'd be happy to look at and merge any PRs though!

from parboiled.

Ok I will give it a try,

from parboiled.

To me this occurred when adding the spotbugs:3.1.12 dependency to a project alongside jtwig:5.87.0 (which uses parboiled:1.1.7)

from parboiled.

@TuomasKiviaho
Do you think that would also help resolve the issue #109 ?

from parboiled.

fwiw, in JDK16 I had to add --add-opens=java.base/java.lang=ALL-UNNAMED.

from parboiled.

I'd be very interested to see a fix for this too. We use Parboiled in Convex (https://convex.world) and it is extremely annoying to always see such warnings. Happy to offer a small crypto bounty of 500 Convex Gold if this can be resolved!

from parboiled.

@TuomasKiviaho
Do you think that would also help resolve the issue #109 ?

It's now been over a year when I patched the project.

The provided gist I believe is backwards compatible with JDK9 since all the bits and bobs needed for JDK compatibility come from ClassInjector of Bytebuddy that I used in the comment itself to call the ParserTransformer directly. Latter part of the code in the comment is just making sure that already injected code isn't injected twice and that the injected code is always returned instead of the original one.

If one is comfy with using the ParserTransformer directly then why not since I did not personally see the need to patch the Parboiled.createParser as well

PS. I remember doing the patch because I needed JDK11 support for my actual parser code (using streams for instance lead to bytecode level problems due to hard-coded in JDK7 output instead of preserving what the original files had) and there the trick that @ben-manes showed would not have helped.

from parboiled.

If anyone would like to submit a PR I'd be more than happy to get it merged and published quickly.

from parboiled.

I'd be happy to see such a PR too, any updates?

from parboiled.

Tried it out right away on a library (GeoTools ) that builds on Java 8, 11, and 17.

On Java 8:

[ERROR] testVariableExpansionWithDefaultColor(org.geotools.styling.css.TranslatorSyntheticTest)  Time elapsed: 0.001 s  <<< ERROR!
java.lang.NoSuchMethodError: java.nio.ByteBuffer.clear()Ljava/nio/ByteBuffer;

Java 11: builds fine

Java 17:

[ERROR] Tests run: 17, Failures: 0, Errors: 17, Skipped: 0, Time elapsed: 0.292 s <<< FAILURE! - in org.geotools.styling.css.CookbookLineTranslationTest
[ERROR] translateTest[zoom.css](org.geotools.styling.css.CookbookLineTranslationTest)  Time elapsed: 0.011 s  <<< ERROR!
java.lang.RuntimeException: Error creating extended parser class: Could not determine whether class 'org.geotools.styling.css.CssParser$$parboiled' has already been loaded
        at org.geotools.styling.css.CookbookLineTranslationTest.<init>(CookbookLineTranslationTest.java:28)
Caused by: java.lang.RuntimeException: Could not determine whether class 'org.geotools.styling.css.CssParser$$parboiled' has already been loaded
        at org.geotools.styling.css.CookbookLineTranslationTest.<init>(CookbookLineTranslationTest.java:28)
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make protected final java.lang.Class java.lang.ClassLoader.findLoadedClass(java.lang.String) accessible: module java.base does not "opens java.lang" to unnamed module @60addb54
        at org.geotools.styling.css.CookbookLineTranslationTest.<init>(CookbookLineTranslationTest.java:28)

from parboiled.

Thank you, @aaime, for this report!
I've built the release on Java 11, which was a mistake apparently.
I should have built on Java 8 instead for publishing the artifacts.

Concerning the problem on Java 17: I'm not sure exactly what the problem is there.
Maybe @SimY4 has an idea? (He contributed the fix for Java 17...)

from parboiled.

Regarding the first problem: to safely release for java 8 target using java version higher it's not enough to set only -source and -target. you have to set the -release flag that was added in Java 9+. It very rarely makes a difference, but in particular, with ByteBuffer and other buffer APIs, it is.

The second one: parboiled requires privileges access to java.lang package - the access that has been restricted ever since the Java module system was added. It's just that in Java 17 this access was hardened even more. But there's still a loophole. You can open up restricted access to java.lang by providing ‘--add-opens java.base/java.lang=ALL-UNNAMED’ as a java runtime parameter.

from parboiled.

Thank you, @SimY4!

Regarding the problem on Java 17:

You can open up restricted access to java.lang by providing ‘--add-opens java.base/java.lang=ALL-UNNAMED’ as a java runtime parameter.

Yes. But I thought that having this addition in the MANIFEST.MF would suffice:

// java 17 patch module path by letting parboiled reflective access to java base package
Compile / packageBin / packageOptions += Package.ManifestAttributes("Add-Opens" -> "java.base/java.lang")

Do the users of parboiled have to add the -add-opens runtime param as well, in addition?

from parboiled.

Do the users of parboiled have to add the -add-opens runtime param as well, in addition?

Unfortunately yes. There's no way around it other than figuring out how not to break java.base module encapsulation. MANIFEST entries only work when parboiled is used as an executable jar. If parboiled is included as a dependency it's not gonna work and --add-opens will be required. I'm sorry, I should probably have told you this earlier.

Link:
http://openjdk.java.net/jeps/261

Quote:

These attributes are interpreted only in the main executable JAR file of an application, i.e., in the JAR file specified to the -jar option of the Java run-time launcher; they are ignored in all other JAR files.

from parboiled.

Can the usages be replaced with public apis? For example ClassLoader.defineClass might be replaced using MethodHandles.Lookup.defineClass.

from parboiled.

Guice's migration might be a helpful example: google/guice@cf759d4

from parboiled.

If there's a clear upgrade path for parboiled, there's also an option to use multi-release jar support that was introduced in Java 9.

This way Java 8 may still use reflective access but in Java 11+ we might use a provided replacement APIs. It's just not very straightforward to replace class loader APIs with method handles.

from parboiled.