Replace most usage of synchronized with ReentrantLock

FasterXML · Mar 15, 2024 · 2668e5f · 2668e5f
1 parent 48124cd
commit 2668e5f
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 16 deletions.
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java
@@ -4,6 +4,7 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.annotation.*;
 
@@ -180,6 +181,8 @@ public abstract class BeanDeserializerBase
      */
     protected transient HashMap<ClassKey, JsonDeserializer<Object>> _subDeserializers;
 
+    private final ReentrantLock _subDeserializersLock = new ReentrantLock();
+
     /**
      * If one of properties has "unwrapped" value, we need separate
      * helper object
@@ -1885,8 +1888,11 @@ protected JsonDeserializer<Object> _findSubclassDeserializer(DeserializationCont
         JsonDeserializer<Object> subDeser;
 
         // First: maybe we have already created sub-type deserializer?
-        synchronized (this) {
+        try {
+            _subDeserializersLock.lock();
             subDeser = (_subDeserializers == null) ? null : _subDeserializers.get(new ClassKey(bean.getClass()));
+        } finally {
+            _subDeserializersLock.unlock();
         }
         if (subDeser != null) {
             return subDeser;
@@ -1902,11 +1908,14 @@ protected JsonDeserializer<Object> _findSubclassDeserializer(DeserializationCont
         subDeser = ctxt.findRootValueDeserializer(type);
         // Also, need to cache it
         if (subDeser != null) {
-            synchronized (this) {
+            try {
+                _subDeserializersLock.lock();
                 if (_subDeserializers == null) {
                     _subDeserializers = new HashMap<ClassKey,JsonDeserializer<Object>>();;
                 }
                 _subDeserializers.put(new ClassKey(bean.getClass()), subDeser);
+            } finally {
+                _subDeserializersLock.unlock();
             }
         }
         return subDeser;

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/DateDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/DateDeserializers.java
@@ -4,6 +4,7 @@
 import java.lang.reflect.Constructor;
 import java.text.*;
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.annotation.JsonFormat;
 
@@ -69,6 +70,8 @@ protected abstract static class DateBasedDeserializer<T>
          */
         protected final DateFormat _customFormat;
 
+        private final ReentrantLock _customFormatLock = new ReentrantLock();
+
         /**
          * Let's also keep format String for reference, to use for error messages
          */
@@ -188,13 +191,16 @@ protected java.util.Date _parseDate(JsonParser p, DeserializationContext ctxt)
                         }
                         return null;
                     }
-                    synchronized (_customFormat) {
+                    try {
+                        _customFormatLock.lock();
                         try {
                             return _customFormat.parse(str);
                         } catch (ParseException e) {
                             return (java.util.Date) ctxt.handleWeirdStringValue(handledType(), str,
                                     "expected format \"%s\"", _formatString);
                         }
+                    } finally {
+                        _customFormatLock.unlock();
                     }
                 }
             }

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/EnumDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/EnumDeserializer.java
@@ -20,6 +20,7 @@
 import com.fasterxml.jackson.databind.util.CompactStringObjectMap;
 import com.fasterxml.jackson.databind.util.EnumResolver;
 import java.util.Optional;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * Deserializer class that can deserialize instances of
@@ -54,6 +55,8 @@ public class EnumDeserializer
      */
     protected volatile CompactStringObjectMap _lookupByToString;
 
+    private final ReentrantLock _lookupLock = new ReentrantLock();
+
     protected final Boolean _caseInsensitive;
 
     private Boolean _useDefaultValueForUnknownEnum;
@@ -472,13 +475,16 @@ protected Class<?> _enumClass() {
     protected CompactStringObjectMap _getToStringLookup(DeserializationContext ctxt) {
         CompactStringObjectMap lookup = _lookupByToString;
         if (lookup == null) {
-            synchronized (this) {
+            try {
+                _lookupLock.lock();
                 lookup = _lookupByToString;
                 if (lookup == null) {
                     lookup = EnumResolver.constructUsingToString(ctxt.getConfig(), _enumClass())
                         .constructLookup();
                     _lookupByToString = lookup;
                 }
+            } finally {
+                _lookupLock.unlock();
             }
         }
         return lookup;

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java
@@ -8,6 +8,7 @@
 import java.net.URI;
 import java.net.URL;
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.io.NumberInput;
@@ -390,6 +391,8 @@ final static class EnumKD extends StdKeyDeserializer
          */
         protected final EnumResolver _byEnumNamingResolver;
 
+        private final ReentrantLock _resolverLock = new ReentrantLock();
+
         protected final Enum<?> _enumDefaultValue;
 
         protected EnumKD(EnumResolver er, AnnotatedMethod factory) {
@@ -472,13 +475,16 @@ private EnumResolver _getToStringResolver(DeserializationContext ctxt)
         {
             EnumResolver res = _byToStringResolver;
             if (res == null) {
-                synchronized (this) {
+                try {
+                    _resolverLock.lock();
                     res = _byToStringResolver;
                     if (res == null) {
                         res = EnumResolver.constructUsingToString(ctxt.getConfig(),
                             _byNameResolver.getEnumClass());
                         _byToStringResolver = res;
                     }
+                } finally {
+                    _resolverLock.unlock();
                 }
             }
             return res;
@@ -495,13 +501,16 @@ private EnumResolver _getToStringResolver(DeserializationContext ctxt)
         private EnumResolver _getIndexResolver(DeserializationContext ctxt) {
             EnumResolver res = _byIndexResolver;
             if (res == null) {
-                synchronized (this) {
+                try {
+                    _resolverLock.lock();
                     res = _byIndexResolver;
                     if (res == null) {
                         res = EnumResolver.constructUsingIndex(ctxt.getConfig(),
                             _byNameResolver.getEnumClass());
                         _byIndexResolver = res;
                     }
+                } finally {
+                    _resolverLock.unlock();
                 }
             }
             return res;

diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeDeserializerBase.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeDeserializerBase.java
@@ -3,6 +3,7 @@
 import java.io.IOException;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 
@@ -42,6 +43,8 @@ public abstract class TypeDeserializerBase
      */
     protected final JavaType _defaultImpl;
 
+    protected final ReentrantLock _defaultImplLock = new ReentrantLock();
+
     /**
      * Name of type property used; needed for non-property versions too,
      * in cases where type id is to be exposed as part of JSON.
@@ -223,12 +226,15 @@ protected final JsonDeserializer<Object> _findDefaultImplDeserializer(Deserializ
             return NullifyingDeserializer.instance;
         }
 
-        synchronized (_defaultImpl) {
+        try {
+            _defaultImplLock.lock();
             if (_defaultImplDeserializer == null) {
                 _defaultImplDeserializer = ctxt.findContextualValueDeserializer(
                         _defaultImpl, _property);
             }
             return _defaultImplDeserializer;
+        } finally {
+            _defaultImplLock.unlock();
         }
     }
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/SerializerCache.java b/src/main/java/com/fasterxml/jackson/databind/ser/SerializerCache.java
@@ -1,6 +1,7 @@
 package com.fasterxml.jackson.databind.ser;
 
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.ser.impl.ReadOnlyClassToSerializerMap;
@@ -45,6 +46,8 @@ public final class SerializerCache
      */
     private final LookupCache<TypeKey, JsonSerializer<Object>> _sharedMap;
 
+    private final ReentrantLock _sharedMapLock = new ReentrantLock();
+
     /**
      * Most recent read-only instance, created from _sharedMap, if any.
      */
@@ -107,29 +110,41 @@ public synchronized int size() {
      */
     public JsonSerializer<Object> untypedValueSerializer(Class<?> type)
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             return _sharedMap.get(new TypeKey(type, false));
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
     public JsonSerializer<Object> untypedValueSerializer(JavaType type)
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             return _sharedMap.get(new TypeKey(type, false));
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
     public JsonSerializer<Object> typedValueSerializer(JavaType type)
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             return _sharedMap.get(new TypeKey(type, true));
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
     public JsonSerializer<Object> typedValueSerializer(Class<?> cls)
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             return _sharedMap.get(new TypeKey(cls, true));
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
@@ -146,29 +161,36 @@ public JsonSerializer<Object> typedValueSerializer(Class<?> cls)
      */
     public void addTypedSerializer(JavaType type, JsonSerializer<Object> ser)
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             if (_sharedMap.put(new TypeKey(type, true), ser) == null) {
                 // let's invalidate the read-only copy, too, to get it updated
                 _readOnlyMap.set(null);
             }
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
     public void addTypedSerializer(Class<?> cls, JsonSerializer<Object> ser)
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             if (_sharedMap.put(new TypeKey(cls, true), ser) == null) {
                 // let's invalidate the read-only copy, too, to get it updated
                 _readOnlyMap.set(null);
             }
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
     public void addAndResolveNonTypedSerializer(Class<?> type, JsonSerializer<Object> ser,
             SerializerProvider provider)
         throws JsonMappingException
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             if (_sharedMap.put(new TypeKey(type, false), ser) == null) {
                 _readOnlyMap.set(null);
             }
@@ -180,14 +202,17 @@ public void addAndResolveNonTypedSerializer(Class<?> type, JsonSerializer<Object
             if (ser instanceof ResolvableSerializer) {
                 ((ResolvableSerializer) ser).resolve(provider);
             }
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
     public void addAndResolveNonTypedSerializer(JavaType type, JsonSerializer<Object> ser,
             SerializerProvider provider)
         throws JsonMappingException
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             if (_sharedMap.put(new TypeKey(type, false), ser) == null) {
                 _readOnlyMap.set(null);
             }
@@ -199,6 +224,8 @@ public void addAndResolveNonTypedSerializer(JavaType type, JsonSerializer<Object
             if (ser instanceof ResolvableSerializer) {
                 ((ResolvableSerializer) ser).resolve(provider);
             }
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 
@@ -213,7 +240,8 @@ public void addAndResolveNonTypedSerializer(Class<?> rawType, JavaType fullType,
             SerializerProvider provider)
         throws JsonMappingException
     {
-        synchronized (this) {
+        try {
+            _sharedMapLock.lock();
             Object ob1 = _sharedMap.put(new TypeKey(rawType, false), ser);
             Object ob2 = _sharedMap.put(new TypeKey(fullType, false), ser);
             if ((ob1 == null) || (ob2 == null)) {
@@ -222,6 +250,8 @@ public void addAndResolveNonTypedSerializer(Class<?> rawType, JavaType fullType,
             if (ser instanceof ResolvableSerializer) {
                 ((ResolvableSerializer) ser).resolve(provider);
             }
+        } finally {
+            _sharedMapLock.unlock();
         }
     }
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java b/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java
@@ -1758,6 +1758,10 @@ protected JavaType _fromVariable(ClassStack context, TypeVariable<?> var, TypeBi
         //
         //    No good way to reproduce but since this should not be on critical path, let's add
         //    syncing as it seems potentially necessary.
+        //
+        // 15-Mar-2024, oddbjornkvalsund: Not replacing this usage of 'synchronized' with an explicit lock as elsewhere.
+        //    Assuming java.lang.reflect.TypeVariable.getBounds() does not do IO or other calls causing the thread to
+        //    park and potentially pin to the platform thread.
         synchronized (var) {
             bounds = var.getBounds();
         }