jackson-modules-base

jackson-modules-base copied to clipboard

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

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

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

Hitting this, too.

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

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.

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?

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

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.

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?

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

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

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

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

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)

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

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

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

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

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

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.

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.

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

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

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

@karussell: That should have worked. I have the following, which is equivalent to what you have (because the setter returns this):

    @NonNull
    private static AfterburnerModule createAfterburnerModule() {
        final AfterburnerModule afterburnerModule = new AfterburnerModule();

        // make Afterburner generate bytecode only for public getters/setter and fields
        // without this, Java 9+ complains of "Illegal reflective access"
        afterburnerModule.setUseValueClassLoader(false);

        return afterburnerModule;
    }

Maybe your issue is not because of Afterburner. Or maybe you use a different object mapper in the code that shows the warning.

Thanks, will investigate!

When I do not load this module at all I was able to remove the warning:

class SomeApp extends Application {
 public void initialize(Bootstrap<GraphHopperServerConfiguration> bootstrap) {
   // this does not load Afterburner at all
   bootstrap.setObjectMapper(Jackson.newMinimalObjectMapper());
   ...
 }
}

(but I get a new warning regarding: Illegal reflective access by com.fasterxml.jackson.databind.util.ClassUtil to constructor java.util.Optional())

@karussell I would be interest in thing about Optional -- it should not be needed, at least if jackson-datatype-jdk8 is included, which uses static factory method. However, if that is not included, databind probably accesses 0-arg constructor just like for any POJO.

I've tried to include this but without success. Maybe I did something wrong? Should I create a different issue to avoid "spamming" here or we can also discuss here: https://github.com/graphhopper/graphhopper/issues/1440

@karussell I believe the trace you pasted in that issue is different. This specifically is related to defining classes, not deserializing Optional types. I recommended a possible fix (make sure jackson-datatype-jdk8 is installed properly everywhere, as Tatu mentions above, but I wish to reiterate that this is 99% likely the cause of your problem!) and if that is not the issue then you may consider opening a new ticket with reproduction steps.

Thanks for the update. I tried a bit around and finally I get no warnings with:

bootstrap.setObjectMapper(Jackson.newMinimalObjectMapper().registerModule(new Jdk8Module()));

For the following I still get the defineClass warning:

bootstrap.setObjectMapper(Jackson.newObjectMapper().registerModule(new AfterburnerModule().setUseValueClassLoader(false)));

Trying to use Jackson.newMinimalObjectMapper():

bootstrap.setObjectMapper(Jackson.newMinimalObjectMapper().registerModule(new AfterburnerModule().setUseValueClassLoader(false)).registerModule(new Jdk8Module()));

does not print a warning but fails with:

java.lang.IllegalAccessError: class com.graphhopper.json.geo.JsonFeatureCollection$Access4JacksonDeserializer2d000ec4 tried to access field com.graphhopper.json.geo.JsonFeatureCollection.features (com.graphhopper.json.geo.JsonFeatureCollection$Access4JacksonDeserializer2d000ec4 is in unnamed module of loader com.fasterxml.jackson.module.afterburner.util.MyClassLoader @302c971f; com.graphhopper.json.geo.JsonFeatureCollection is in unnamed module of loader 'app')