Merge branch '2.5'

Conflicts: release-notes/CREDITS
FasterXML · Mar 28, 2015 · 6c5826c · 6c5826c
2 parents 4e34ea1 + ec1820d
commit 6c5826c
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 52 deletions.
diff --git a/release-notes/CREDITS b/release-notes/CREDITS
@@ -216,6 +216,11 @@ 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)
+
 Charles Allen (drcrallen@github):
   * Reported #696: Copy constructor does not preserve `_injectableValues`
    (2.6.0)
diff --git a/release-notes/VERSION b/release-notes/VERSION
@@ -31,6 +31,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
 - Refactor `TypedKey` into separate util class

diff --git a/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java b/src/main/java/com/fasterxml/jackson/databind/SerializerProvider.java
@@ -1112,12 +1112,13 @@ protected JsonSerializer<Object> _findExplicitUntypedSerializer(Class<?> runtime
      * Method that will try to construct a value serializer; and if
      * one is successfully created, cache it for reuse.
      */
-    protected JsonSerializer<Object> _createAndCacheUntypedSerializer(Class<?> type)
+    protected JsonSerializer<Object> _createAndCacheUntypedSerializer(Class<?> rawType)
         throws JsonMappingException
-    {        
+    {
+        JavaType type = _config.constructType(rawType);
         JsonSerializer<Object> 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
@@ -1156,8 +1157,15 @@ protected JsonSerializer<Object> _createAndCacheUntypedSerializer(JavaType type)
     protected JsonSerializer<Object> _createUntypedSerializer(JavaType type)
         throws JsonMappingException
     {
-        // 17-Feb-2013, tatu: Used to call deprecated method (that passed property)
-        return (JsonSerializer<Object>)_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<Object>)_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
@@ -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 = 2000;
+        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<String> writeJson = new Callable<String>() {
             @Override
@@ -120,17 +88,12 @@ public String call() throws Exception {
             JsonNode wrapped = tree.get("hasSubTypes");
 
             if (!wrapped.has("one")) {
-                throw new IllegalStateException("Missing 'one', source: "+json);
+                throw new IllegalStateException("Round #"+round+"/"+max+" ; missing property 'one', source: "+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();
     }
 }