Java 9 warning for Afterburner

[0x9090]

When compiling a Dropwizard project with Java 9, Jackson compilation throws the following error,

WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by com.fasterxml.jackson.module.afterburner.util.MyClassLoader (file:/Users/user/Code/application.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int) WARNING: Please consider reporting this to the maintainers of com.fasterxml.jackson.module.afterburner.util.MyClassLoader 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

[cowtowncoder]

Thank you. I have no idea what that means, or what could be done. But it's good to have here for future reference.

[0x9090]

Looks like a common symptom is the use of internal packages. Could be what's going on here. It doesn't seem critical, but something to consider as more stuff moves to Java 9.

https://blog.codefx.org/java/java-9-migration-guide/#Illegal-Access-To-Internal-APIs

[black-snow]

Hitting this, too.

[pphilion-isp]

I've seeing this too.

It's related to the calls in loadAndResolve(ClassName className, byte[] byteCode) in MyClassLoader, specifically the defineClass override (I'm not sure which one, I'll see if testing catches it).

[cowtowncoder]

Ok. That's the "meat" of the system... so can not be ignored or skipped.

The part that is bit troubling is that it seems plausible fix could only be done in Java 9. And even Jackson 3.0 will only be Java 8 based.

Or maybe there is simpler way. I suspect if so, that would still need to go in Jackson 3.0, where bigger rewrites are possible.

[pphilion-isp]

Confirming my understanding: Jackson 3.0 will still be Java 8 based.

Should I avoid deploying under Java 10, or can a Java 8 Jackson service fat jar run fine in a Java 10 Docker container?

[cowtowncoder]

@istream-philion Jackson requirement simply means that Jackson will only require Java 8 platform, not that it would not run on a later one. Components have been tested on Java 9 to some degree (all unit tests pass, as far as I know), and there are no known reasons why they couldn't work on Java 10.
But it is possible there are some issues that would be uncovered. It is less likely there would be problems for core components (streaming API should definitely work with no problem; databind should work too, the only concern being visibility access for introspection).

[jsjohnst]

Adding this gets me past the warning...

--add-opens java.base/java.lang=ALL-UNNAMED

Still needs a better solution though, but its a start.

[cristian-mocanu-mob]

The problem is that Afterburner uses internal JDK API: the method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int) is protected, and Oracle may simply removed it in even in a minor release.

Maybe Afterburner could use something else for bytecode generation, like ASM, JavaAssist, or Janino?

[cowtowncoder]

@cristian-mocanu-mob Afterburner uses ASM (for 2.x) and ByteBuddy (3.x). But those just generate bytecode, one still has to load the class in JVM. But if libraries offer help there, that'd be great.

I mean usage is literally just "load the class as per bytecode generate", there is no magic. I just haven't followed up on JVM developments in this area. It could be some simple change.

[jsjohnst]

@cowtowncoder Would something like this work (I’ve not done it, so I’m not sure)?

https://analyzejava.wordpress.com/2014/09/25/java-classloader-write-your-own-classloader/

[pphilion-isp]

I think the issue is relates to the block that starts at https://github.com/FasterXML/jackson-modules-base/blob/master/afterburner/src/main/java/com/fasterxml/jackson/module/afterburner/util/MyClassLoader.java#L81

Specifically, the warning calls out illegal reflection.

I don't understand the block at all. Why use reflection when just calling defineClass() should work?

Aside: I don't think it is the protected method that's the issue. That's a standard interface on a system class: even though is it protected, any class can subclass and invoke. It's not private, or an non-standard class (or even deprecated, there is an older defineClass).

[cowtowncoder]

@jsjohnst That article predates Java 9, so it wouldn't cover it. But actually as per @istream-philion there's bit more here; simple use case presented works but is not what module always does.

@istream-philion Ok so the comment suggests that this would be necessary for the case where class to access (or accessors) is not public. In those cases it is necessary to call defineClass() of the parent class loader of the value class being optimized: only that is able to access such members (and only when accessor class is in same package).
(it took a while to reverse-engineer this since getParent() could mean various other things...)

So: getting rid of this access would make it impossible to optimize access to any non-public field of any non-public class. And that would be quite unfortunate.

[cowtowncoder]

Ok: and just to clarify above... reflection is needed because defineClass() is protected and although sub-class can call it on this, it can not call that method on any other ClassLoader instance. And that's the crux. I can see how this goes against access restrictions Java 9's module system imposes, but if worst comes to worst, bytecode generation for accessors will be limited to public classes and their public members. And it is possible to configure module to do that already (see config in AfterburnerModule)

[cristian-mocanu-mob]

@cowtowncoder: am I correct that, to disable bytecode generation for private members, I would need to do AfterburnerModule.setUseValueClassLoader(false)?

[cowtowncoder]

@cristian-mocanu-mob I would have to verify exact behavior here, but I think that private accessor are skipped in either case, and this setting would only affect handling of protected and "package protected" access cases.
This setting does affect whether Afterburner tries to access ClassLoader of value class itself, to access such non-public accessors, if (and only if) needed.

[cristian-mocanu-mob]

Setting AfterburnerModule.setUseValueClassLoader(false) fixed all warnings for me.
Since I am only mapping public getters/setters/constructors, this causes no issue for me.

[re-thc]

https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/MethodHandles.Lookup.html#defineClass-byte:A- is the replacement method (previously via unsafe)

[cowtowncoder]

@hc-codersatlas Thank you for suggesting this.

This will be a pain no matter what, since Jackson can not use Java 9 features directly with 2.x at all (ever). 3.x may at some point, but initially plan is only to require Java 8.

But as long as it's just one method we can probably make it work dynamically, to allow Java 8 and Java 9+ both work with different implementation, dynamically loaded.

[stevenschlansker]

Have you considered targeting jdk9 or 10 for Jackson 3? 8 is going to be EOL very soon, and being 9-native would be a nice feature.

[cowtowncoder]

No. I don't use Java 9 myself, seems like more pain than value at this point. But when there is next long-term version (11?), that might change, within 3.x.

[re-thc]

11 is out in September and 3.x might not be out until then? i.e. might be worth considering.

[karussell]

@cristian-mocanu-mob how did you get rid of the warning? I tried the following without success:

objectMapper.registerModule(new AfterburnerModule().setUseValueClassLoader(false));

[alibenmessaoud]

Same warning messages. I use Dropwizard 1.3.12 on Java 11.

[Sounie]

I wonder if multi JVM version support could be smoothly introduced by applying
Multi-Release JAR Files

Keep the "root" code as compiled for Java 8, and introduce an alternative implementation for the ClassLoader reflection for Java 9+ to pick up.

It will probably introduce some more complexity to the build process, but this seems to be a good match for what the JEP is intended to facilitate.

--
Stephen

@hc-codersatlas Thank you for suggesting this.

This will be a pain no matter what, since Jackson can not use Java 9 features directly with 2.x at all (ever). 3.x may at some point, but initially plan is only to require Java 8.

But as long as it's just one method we can probably make it work dynamically, to allow Java 8 and Java 9+ both work with different implementation, dynamically loaded.

[OrangeDog]

I have no idea what that means

In case it wasn't clear yet - Afterburner is using reflection to access a non-public API outside of its own module (which in this case is "all non-module code"), that may behave differently (or in general, be missing) in any given JDK implementation (across both vendor and version).

While it does seem unlikely that a protected method would break, there is no guarantee that something bad wouldn't happen if this was called from outside a properly-conforming subclass of ClassLoader.

[cowtowncoder]

At this point discussion on this issue, and its original vague wording mean that it is difficult to follow.
Reporting yet more "hey I run DropWizard and see a warning" will not help; and due to changes in module-status of other libraries (i.e. which of one's dependencies are full modules, use automatic module name, or none of above) change symtoms.
So basically this is not really actionable at this point.

So: I think new, more specific issue(s) should be filed, with possible reference to this issue as background/context.

I will probably close this one soon.

[stevenschlansker]

One possible route of exploration to maintain current Afterburner functionality is to change from Unsafe.defineClass to MethodHandles.Lookup.defineClass. This has the possibility of preserving the existing ability to "crack open" non-public types.

[cowtowncoder]

@stevenschlansker There are couple of other issues: would that approach fall under them? Or a new issue? Since MethodHandles is available on JDK 7 that seems like possibility (plus I would be ok with Afterburner going Java 8 even during Jackson 2.x).

[stevenschlansker]

I haven't tried! Just leaving it as a suggestion for follow-up work in case anyone really wants to update Afterburner proper. We are happily running the Blackbird pre-release and have not looked back :)

[cowtowncoder]

@stevenschlansker ah right. One more question (which I may have asked previously but forgot answer to...) : does each MethodHandle typically generate a single class instance, similar to how a lambda would work?

[cogman]

@cowtowncoder I didn't see that you got a response here (Don't know if it came offline). But it is my understanding that no new classes are created for method handles/references.

The source is complex around it, but from my cursory glance, it does not look like any classes are being introduced.

AFAIK, exact behavior isn't specified (so YMMV with other JVMs)

At least in OpenJDK, it looks like they are going to great lengths to make sure the returned MethodHandle (if you "unreflect" a method) translates basically directly into the right invocation instruction (invoke special/static/etc). That means these should be slow to create but really fast to use.

[cogman]

Also of note, if you are willing to bump the requirement up to Java 9, then you can use VarHandle to do to object fields what you are doing to object methods. I don't know if that is out of the question, though, since there appears to be a lot of people out there stuck on 8.

[cowtowncoder]

If no classes are loaded in memory, that'd be a significant plus I think.

O Java 9, that would not be an option for 2.x, but by the time 3.0 is out, perhaps 11 or 14 (one of LTS versions) could become baseline. At least for Afterburner (although whether one can use post-Java8 JDK for generating later versions and java 8 in same repo I don't know -- Oracle has not been very supportive of doing java 8 builds from later versions AFAIK).

[wrprice]

Please pardon my lurking, and I hope I understand the concern correctly...

although whether one can use post-Java8 JDK for generating later versions and java 8 in same repo I don't know

OpenJDK 11 javac with the --release 8 flag will emit JDK8-compatible bytecode just fine. I've been doing it w/ a production codebase for years. The trickier part is modifying your build scripts to make multi-release JAR files. You'd build the majority of the project using javac 11 and --release 8, then "layer" Java 11 classes on top with a second javac step (without --release) that builds alternate versions of only those classes that want to use JDK9+ features. The APIs of the alternate versions should match the Java 8 versions, but the implementation details can differ. Place the output of the first compilation step on the classpath for the second step so that the Java 11 alternates properly link to any shared/common Java 8 classes.

Then run the test suite twice, once each with JDK8 and JDK11.

[cowtowncoder]

Right, I don't think I'd really want to develop or support true multi-release jars in that sense (although it is good that is possible), but rather isolate support for later JDK versions into distinct modules.
But it may be necessary to build different maven sub-projects with different JDK settings, which should be doable.

[OrangeDog]

Given that blackbird is now a core module, is there any intention to fix afterburner, or should users just switch?

[cowtowncoder]

If someone can suggest or contribute a fix I'd be happy to merge it, but I am not aware of one.
Given this, yes, it probably makes sense for post-Java-8 users to have a look at Blackbird. My only concern there is that there has been less real world usage, but there is only one way to get over that.

[eperret]

Is this still an issue in versions of Java later than 9?