From 48617c885ec1c2563458d5d66ed637a4972dc1cc Mon Sep 17 00:00:00 2001 From: Cowtowncoder Date: Fri, 27 Mar 2015 14:39:20 -0700 Subject: [PATCH 1/2] Simplify failing test --- .../jackson/failing/RaceCondition738Test.java | 59 ++++--------------- 1 file changed, 12 insertions(+), 47 deletions(-) diff --git a/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java b/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java index 20e67a57c9..e296b38cd7 100644 --- a/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java +++ b/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java @@ -8,16 +8,9 @@ import com.fasterxml.jackson.databind.BaseMapTest; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.module.SimpleModule; public class RaceCondition738Test extends BaseMapTest { - @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.WRAPPER_OBJECT) - @JsonSubTypes({ - @JsonSubTypes.Type(value = TypeOne.class, name = "one"), - @JsonSubTypes.Type(value = TypeTwo.class, name = "two"), - @JsonSubTypes.Type(value = TypeThree.class, name = "three") - }) static abstract class AbstractHasSubTypes implements HasSubTypes { } static class TypeOne extends AbstractHasSubTypes { @@ -35,36 +28,10 @@ public String getType() { } } - static class TypeTwo extends AbstractHasSubTypes { - private final String id; - public TypeTwo(String id) { - this.id = id; - } - @JsonProperty - public String getId() { - return id; - } - @Override - public String getType() { - return TypeTwo.class.getSimpleName(); - } - } - - static class TypeThree extends AbstractHasSubTypes { - private final String id; - public TypeThree(String id) { - this.id = id; - } - @JsonProperty - public String getId() { - return id; - } - @Override - public String getType() { - return TypeThree.class.getSimpleName(); - } - } - + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.WRAPPER_OBJECT) + @JsonSubTypes({ + @JsonSubTypes.Type(value = TypeOne.class, name = "one") + }) public interface HasSubTypes { String getType(); } @@ -89,12 +56,13 @@ public HasSubTypes getHasSubTypes() { */ public void testRepeatedly() throws Exception { - for (int i = 0; i < 1000; i++) { - runOnce(); + final int COUNT = 50; + for (int i = 0; i < COUNT; i++) { + runOnce(i, COUNT); } } - void runOnce() throws Exception { + void runOnce(int round, int max) throws Exception { final ObjectMapper mapper = getObjectMapper(); Callable writeJson = new Callable() { @Override @@ -120,17 +88,14 @@ public String call() throws Exception { JsonNode wrapped = tree.get("hasSubTypes"); if (!wrapped.has("one")) { - throw new IllegalStateException("Missing 'one', source: "+json); +System.out.println("JSON wrong: "+json); + throw new IllegalStateException("Round #"+round+"/"+max+" ; missing property 'one', source: "+json); } +System.out.println("JSON fine: "+json); } } private static ObjectMapper getObjectMapper() { - SimpleModule module = new SimpleModule("subTypeRace"); - module.setMixInAnnotation(HasSubTypes.class, AbstractHasSubTypes.class); - - ObjectMapper mapper = new ObjectMapper(); - mapper.registerModule(module); - return mapper; + return new ObjectMapper(); } } From ec1820d591bbb51edb96942444d1806e2b86bc12 Mon Sep 17 00:00:00 2001 From: Cowtowncoder Date: Fri, 27 Mar 2015 16:55:52 -0700 Subject: [PATCH 2/2] Fix #738 --- release-notes/CREDITS | 5 +++++ release-notes/VERSION | 2 ++ .../jackson/databind/SerializerProvider.java | 18 +++++++++++++----- .../jackson/failing/RaceCondition738Test.java | 4 +--- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/release-notes/CREDITS b/release-notes/CREDITS index 03859ad4e7..087e6279dd 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -215,3 +215,8 @@ Antibrumm@github: Francisco A. Lozano (flozano@github) * Contributed fix for #703 (see above) (2.5.2) + +Dylan Scott (dylanscott@github) + * Reported #738: #738: @JsonTypeInfo non-deterministically ignored in 2.5.1 (concurrency + issue) + (2.5.2) diff --git a/release-notes/VERSION b/release-notes/VERSION index 2c5620519d..8a8525472b 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -19,6 +19,8 @@ Project: jackson-databind (reported by jkochaniak@github) #733: MappingIterator should move past errors or not return hasNext() == true (reported by Lorrin N, lorrin@github) +#738: @JsonTypeInfo non-deterministically ignored in 2.5.1 (concurrency issue) + (reported by Dylan S, dylanscott@github) - Improvement to handling of custom `ValueInstantiator` for delegating mode; no more NPE if `getDelegateCreator()` returns null diff --git a/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java b/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java index 24a1b77261..f45f8a425e 100644 --- a/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java +++ b/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java @@ -1111,12 +1111,13 @@ protected JsonSerializer _findExplicitUntypedSerializer(Class runtime * Method that will try to construct a value serializer; and if * one is successfully created, cache it for reuse. */ - protected JsonSerializer _createAndCacheUntypedSerializer(Class type) + protected JsonSerializer _createAndCacheUntypedSerializer(Class rawType) throws JsonMappingException - { + { + JavaType type = _config.constructType(rawType); JsonSerializer ser; try { - ser = _createUntypedSerializer(_config.constructType(type)); + ser = _createUntypedSerializer(type); } catch (IllegalArgumentException iae) { /* We better only expose checked exceptions, since those * are what caller is expected to handle @@ -1155,8 +1156,15 @@ protected JsonSerializer _createAndCacheUntypedSerializer(JavaType type) protected JsonSerializer _createUntypedSerializer(JavaType type) throws JsonMappingException { - // 17-Feb-2013, tatu: Used to call deprecated method (that passed property) - return (JsonSerializer)_serializerFactory.createSerializer(this, type); + /* 27-Mar-2015, tatu: Wish I knew exactly why/what, but [databind#738] + * can be prevented by synchronizing on cache (not on 'this', however, + * since there's one instance per serialization). + * Perhaps not-yet-resolved instance might be exposed too early to callers. + */ + synchronized (_serializerCache) { + // 17-Feb-2013, tatu: Used to call deprecated method (that passed property) + return (JsonSerializer)_serializerFactory.createSerializer(this, type); + } } /** diff --git a/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java b/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java index e296b38cd7..aca5fe8b02 100644 --- a/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java +++ b/src/test/java/com/fasterxml/jackson/failing/RaceCondition738Test.java @@ -56,7 +56,7 @@ public HasSubTypes getHasSubTypes() { */ public void testRepeatedly() throws Exception { - final int COUNT = 50; + final int COUNT = 2000; for (int i = 0; i < COUNT; i++) { runOnce(i, COUNT); } @@ -88,10 +88,8 @@ public String call() throws Exception { JsonNode wrapped = tree.get("hasSubTypes"); if (!wrapped.has("one")) { -System.out.println("JSON wrong: "+json); throw new IllegalStateException("Round #"+round+"/"+max+" ; missing property 'one', source: "+json); } -System.out.println("JSON fine: "+json); } }