Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@JsonTypeInfo non-deterministically ignored in 2.5.1 #738

Closed
dylanscott opened this issue Mar 27, 2015 · 9 comments
Closed

@JsonTypeInfo non-deterministically ignored in 2.5.1 #738

dylanscott opened this issue Mar 27, 2015 · 9 comments
Milestone

Comments

@dylanscott
Copy link

We're observing what appears to be a race condition in Jackson 2.5.1, where a particular use of @JsonTypeInfo will non-deterministically get ignored when writing JSON. I have a small test case which for me reproduces the issue 30-50% of the time. We're copying the structure of a type in our application, which looks like the following:

          HasSubTypes (interface)
               |
       AbstractHasSubTypes (uses @JsonTypeInfo and @JsonSubTypes, serializing as wrapper object)
       /       |         \
TypeOne     TypeTwo     TypeThree

We mixin AbstractHasSubTypes' annotations with HasSubTypes, and then serialize a Wrapper object which has a single field of type HasSubTypes. There appears to be a race condition when serializing these wrapper objects in parallel, where some percentage of the time the @JsonTypeInfo information is completely ignored and we don't serialize any type information. The linked test case should hopefully make this clearer, it's pretty small. This issue does not appear to affect 2.4.3.

@sfackler
Copy link

cc #709

@cowtowncoder
Copy link
Member

I am investigating possible cause, related to #735 -- speculating that perhaps inappropriate caching could lead to this problem. This sounds plausible given that #735 also started in 2.4.4.
I am not sure if fix for #735 would solve this issue, but it is perhaps worth checking: jackson-databind 2.4.5.1 ("micro-patch") was released yesterday, so it may be easy to see if that makes a difference.

Similar fix is also merge in 2.5 (to be included in 2.5.2), but I want to add more testing for Map and Collection deserialization first to ensure that the problem regarding container types is properly resolved.

@sfackler
Copy link

I'm seeing it in 2.5.1, 2.5.0, and 2.5.0-rc1, but not any of 2.4.5, 2.4.4, or 2.4.3.

@cowtowncoder
Copy link
Member

@sfackler Ok, thanks! I can reproduce this locally, with similar frequency.

Also fix for #735 had no effect here, plus this one appears to be on serialization side. So much for my suspicion that these were related.

cowtowncoder added a commit that referenced this issue Mar 27, 2015
@cowtowncoder
Copy link
Member

Looks like test can be further simplified: mix-in is not necessary, and can remove alternate sub-classes. Seems to also fail relatively fast, if at all.

@cowtowncoder
Copy link
Member

Ok. I was able to figure out a place in code where additional synchronization can prevent the failure.
What bothers me is that I do not yet fully understand, end to end, exactly what the problem is, and why synchronization (basic on per-ObjectMapper object; in this case, SerializerCache) prevents it.
I think that instead of incorrect caching it is due to exposing partially resolved instance too early (meaning that the incorrect output would be sort of transient), but I can not fully verify this.

Nonetheless due to severity of the problem, and since fix does work for the test case given (with longer run times; compared to regularly failing without sync block), and since there should not be much actually harm (code should not be called often once caching takes effect), I think I will add the fix/work-around, get 2.5.2 released, and just keep this in mind for future.

@alexo
Copy link

alexo commented Jun 3, 2015

+1 I'm able to reproduce this issue in our test environment with version 2.5.1.
When a new version will be released?

@cowtowncoder
Copy link
Member

@alexo 2.5.3 is the latest released version from 2.5.x branch, so you should be able to upgrade.

@alexo
Copy link

alexo commented Jun 4, 2015

@cowtowncoder thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants