IllegalAccessException on final field

[smeder]

I'm getting the following exception:

WARN [2014-08-18 13:57:17,088] com.kosei.dropwizard.advertiser.api.JsonOrganization$Access4JacksonDeserializer: Disabling Afterburner deserialization for type class com.kosei.dropwizard.advertiser.api.JsonOrganization, field #0, due to access error (type java.lang.IllegalAccessError, message=null)

! java.lang.IllegalAccessError: null
! at com.kosei.dropwizard.advertiser.api.JsonOrganization$Access4JacksonDeserializer.objectField(com.kosei.dropwizard.advertiser.api.JsonOrganization$Access4JacksonDeserializer.java) ~[na:0.16]
! at com.fasterxml.jackson.module.afterburner.deser.BeanPropertyMutator.objectField(BeanPropertyMutator.java:142) ~[jackson-module-afterburner-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.module.afterburner.deser.SettableObjectFieldProperty.set(SettableObjectFieldProperty.java:58) [jackson-module-afterburner-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.databind.deser.impl.PropertyValue$Regular.assign(PropertyValue.java:62) [jackson-databind-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:173) [jackson-databind-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:398) [jackson-databind-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1056) [jackson-databind-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.module.afterburner.deser.SuperSonicBeanDeserializer.deserializeFromObject(SuperSonicBeanDeserializer.java:190) [jackson-module-afterburner-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:124) [jackson-databind-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.module.afterburner.deser.SuperSonicBeanDeserializer.deserialize(SuperSonicBeanDeserializer.java:109) [jackson-module-afterburner-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.databind.ObjectReader._bind(ObjectReader.java:1232) [jackson-databind-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:676) [jackson-databind-2.4.2.jar:2.4.2]
! at com.fasterxml.jackson.jaxrs.base.ProviderBase.readFrom(ProviderBase.java:808) [jackson-jaxrs-base-2.4.2.jar:2.4.2]

when deserializing to

@JsonInclude(JsonInclude.Include.NON_NULL)
public class JsonOrganization extends Resource<JsonOrganization> {
    public static final String ORGANIZATION_LINK_NAME = "organization";
    public final long id;
    public final String name;
    public final JsonAddress address;

    @JsonCreator
    public JsonOrganization(@JsonProperty("id") long id,
                            @JsonProperty("name") String name,
                            @JsonProperty("address") JsonAddress address)
    {
        this.id = id;
        this.name = name;
        this.address = address;
    }
}

I am assuming that is not expected? Plain Jackson deserializes it without problems.

[cowtowncoder]

Interesting. Thank you for providing the test case, and yes, things should work functionally the same way with or without Afterburner, so this is a bug.

[cowtowncoder]

Unfortunately I am not able to reproduce your problem just by using included types, simple test instance.

[smeder]

Hmm, ok. Going to dig in a bit more. This is happening for REST calls in Dropwizard (0.7.1), so I'm not setting up the mapper myself. I'll let you know what I find (might be a few days)

[cowtowncoder]

@smeder Ok thanks. I just mentioned oddities I observed wrt final members, since it sounds like this problem might be quite context-sensitive. For example, only occurring for child objects, or for parents, or something with both parent and children.

[smeder]

Finally got around to looking into this again. I put together a very simple test case that triggers it every time for me: https://gist.github.com/smeder/485f31cdd339691ca355

In case it matters, I am running on Java 8:

java version "1.8.0_05"
Java(TM) SE Runtime Environment (build 1.8.0_05-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.5-b02, mixed mode)

[cowtowncoder]

Hmmh. And it passes on my local 1.7 build...

[cowtowncoder]

Also passes with 1.8 locally. I run on MacOS, for what that is worth.

[smeder]

Hmm, ok. I'll have to play around a bit more with my environment to see if I can isolate what is causing the different behavior.

[bimargulies]

I've hit this. Let me see if I can abstract to a test case.

[cowtowncoder]

Ok. In general, I think I'd suggest NOT trying to inject final fields for anything but "leaf" types (ones that do not refer to structured types). It appears there is some magical limit on how long final fields are actually mutable (to allow JDK serialization, as far as I know); and looks like that limit may be triggered quite easily.

Perhaps there is a way around this, but just something to keep in mind when working around issues.

[bimargulies]

@cowtowncoder

https://github.com/bimargulies/afterburner-tc is a test case for you. There's a standard reflection trick to disable final protection on non-static fields, is there not? In my test case, there's a registered constructor creator, so the problem of final should not be a problem. In short, my test case may give you two problems: 1: afterburner ignoring a mixin with a JsonCreator, and 2: the final problem.

[smeder]

From the discussion I guess I don't really understand how this is supposed to work. Why do you need to mutate final fields when there is a JsonCreator annotated constructor?

[smeder]

Ok, updated my test code (https://gist.github.com/smeder/485f31cdd339691ca355) and realized that it was triggering because the Resource super class has a property that is not being included in the constructor. I'm guessing that this triggers a fallback mechanism that tries to mutate the final fields? Is there any way to mark a field as read only?

[bimargulies]

BINGO: when I cleaned up and completed my mixin constructors, it all works. I'll push the fix so thart @smeder can look.

[bimargulies]

@cowtowncoder is there any way to imagine a module that would just have the job of logging gaps in @JsonCreator coverage? Or a way to disable poking fields so that tests without the afterburner would fail?

[cowtowncoder]

@bimargulies If there is a trick, I am not aware of that. But come to think of it, core Jackson simply uses Reflection, and Afterburner itself should not even try to change that (since it can't really do it without reflection anyway).

[cowtowncoder]

@smeder I may be conflating two separate issues here -- I remember problems with final fields. But you are right, it should not be relevant here at all.

Except, right, when that field is mutated; although that should seemingly also occur normally without Afterburner.

However: this is where I guess Afterburner could affect things -- whereas core databind ONLY uses reflection, Afterburner mixes actual calls, so perhaps it is method calls that trigger end of time during which final fields may be modified. Or something along those lines.

Or. Actually. A more likely scenario is perhaps this: ability to change values of final fields is only available if Object itself is constructed using Reflection. This is what core databind does. But Afterburner instead uses regular constructor, not reflection; and after this, final fields are finalized.
Now, for non-final, non-private fields, Afterburner will also "inline" field access. But for final or private fields, Reflection access is the default to use.

If above hypothesis is correct, it is quite difficult to actually prevent this problem.
Not necessarily impossible, but challenging.

[cowtowncoder]

@bimargulies There is no assumption that @JsonCreator would provide values to all fields; that is, it is perfectly fine to provide some properties with creator, others via mutators (setters, fields).

Having said that, I am open to suggestions on adding ways to indicate that creator should be "complete", that is, existence of properties outside of creators is considered invalid.
This could be by, say, a new property for @JsonCreator, like... mustBeComplete or complete?

[smeder]

Hmm, how about a way to mark a property as read only instead?

[cowtowncoder]

Ok, one more suggestion: if may make sense to disable auto-detection of fields completely. By default fields of any visibility (including private) may be auto-detected for purpose of setting value. But in these example cases this is not desired. So using @JsonAutoDetect, or setting default visibility for ObjectMapper would achieve the goal of never using private fields at all.

@smeder Read-only is achieved by not having any way to set the value. This can be done by explicitly marking fields with @JsonIgnore (but if so, also ensuring that @JsonProperty is used for getter for constructor parameters). But visibility setting above should also achieve this.

I hope visibility change works here, since it is also something I think you actually want to do anyway, but that just has not mattered before (i.e. it has been there all along, without needing to be, and now being harmful).

[smeder]

I actually want things to be visible for serialization. As far as I can tell @JsonAutoDetect applies to both serialization & deserialization, so I don't think this would work by itself. I'm basically trying to achieve asymmetric serialization & deserialization behavior.

I know my immutable "struct" like classes are a bit weird as far as Java code is concerned (I don't really see the point for bean methods on these kind of classes), but it would be nice if there was a solution that didn't involve @JsonIgnore + @JsonProperty annotated getter. Seems like asymmetric use case comes up once in a while so it would be nice if @JsonIgnore had flags for indicating whether to ignore for serialization, deserialization or both.

Having said all of that, I am not sure that what I am doing is actually the right approach, so the whole issue may be moot for me.

[cowtowncoder]

Not necessarily. Auto-detection applies to different accessors separately, and although fields may be used for serialization and deserialization, typically they are often either not used at all (unless someone forgets to include setter or constructor argument), or are only used for deserialization.
So it is quite possible to define visibility so that field access is not used.

The problem with trying to add property to @JsonProperty is that there is more complex interaction between accessors (individual setter/getter/field), and combined logical property.

One thing, however, that might make sense is to have a new DeserializationFeature, like ALLOW_FINAL_FIELDS_AS_SETTER (or such), disabling of which just made sure that Jackson never tries to use such fields as setters. In fact, that they are used is more an unintended feature, as I did not realize that (a) it is being used and that (b) it actually works. However, given that this is the case I am certain someone somewhere is relying on this feature.

If this idea (on new feature) makes sense, please file it for jackson-databind and it should be quite easy to add for 2.5.

[smeder]

I played around a bit with auto-detection. When I turn off field detection I basically have to annotate every field with @JsonProperty for things to work. It works, but isn't great.

Did you mean @JsonIgnore in your second paragraph?

I think the DeserializationFeature would make sense. I'm assuming that in that case the @JsonCreator annotated constructor would be used for deserialization and json fields not represented in the constructor would be treated as unknown?

[cowtowncoder]

Right, it may be necessary to use @JsonProperty. I am not 100% sure if simple visibility rules would work without it -- there are many related rules in determining what should be visible in order to minimize need for annotations for common case.
I'll actually file an issue for jackson-databind to check this, and improve handling if possible.

As to feature, yes, it would drop usage of final fields for setting values; if setters exist, those would still be used (since finality of setter has no relation to immutability of logical property). Also, just because field 'x' is final does not necessarily mean setX(..) should not be used, as there is no assumption that field and setter are always linked -- often they aren't (i.e. setter has some other logic to use).

This feature could probably be used together with, or as an alternative to, proposed "this creator is complete, no other setters should be used" addition to @JsonCreator, depending on situation.

[smeder]

Ok, I'll file a request for the jackson-databind module. Also wanted to say that I really appreciate the time you've spent on this.

[cowtowncoder]

No problem; thank you for discussing it through; I think I finally understand low level details of what is going on.

FWIW, I also filed this: FasterXML/jackson-databind#563 to see if visibility handling is working the way it is intended to.

[cowtowncoder]

@smeder Actually one feature I had forgotten about is MapperFeature.INFER_PROPERTY_MUTATORS -- if it is disabled (enabled by default), private fields will NOT be used as setters, unless explicitly annotated, or visibility limit is increased to make them visible.

So you will probably want to disable this, just in case.

And finally, I will try to add a reproducible unit test here as the very first thing.

[cowtowncoder]

Ok. Also, as to the original trigger, Afterburner was not verifying whether field to access is final.
Now it will do that and avoid triggering this particular problem.