Use ReentrantLocks instead of synchronized blocks - v2.17.1 (#186)

FasterXML · Apr 5, 2024 · f81ffc2 · f81ffc2
1 parent 47083ea
commit f81ffc2
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 55 deletions.
diff --git a/base/src/main/java/com/fasterxml/jackson/jaxrs/cfg/MapperConfiguratorBase.java b/base/src/main/java/com/fasterxml/jackson/jaxrs/cfg/MapperConfiguratorBase.java
@@ -81,27 +81,27 @@ public MapperConfiguratorBase(MAPPER mapper, Annotations[] defaultAnnotations)
     /***********************************************************
      */
 
-    public synchronized final void setMapper(MAPPER m) {
+    public final void setMapper(MAPPER m) {
         _mapper = m;
     }
 
-    public synchronized final void setAnnotationsToUse(Annotations[] annotationsToUse) {
+    public final void setAnnotationsToUse(Annotations[] annotationsToUse) {
         _setAnnotations(mapper(), annotationsToUse);
     }
 
-    public synchronized final void configure(DeserializationFeature f, boolean state) {
+    public final void configure(DeserializationFeature f, boolean state) {
         mapper().configure(f, state);
     }
 
-    public synchronized final void configure(SerializationFeature f, boolean state) {
+    public final void configure(SerializationFeature f, boolean state) {
         mapper().configure(f, state);
     }
 
-    public synchronized final void configure(JsonParser.Feature f, boolean state) {
+    public final void configure(JsonParser.Feature f, boolean state) {
         mapper().configure(f, state);
     }
 
-    public synchronized final void configure(JsonGenerator.Feature f, boolean state) {
+    public final void configure(JsonGenerator.Feature f, boolean state) {
         mapper().configure(f, state);
     }
 

diff --git a/cbor/src/main/java/com/fasterxml/jackson/jaxrs/cbor/CBORMapperConfigurator.java b/cbor/src/main/java/com/fasterxml/jackson/jaxrs/cbor/CBORMapperConfigurator.java
@@ -1,6 +1,7 @@
 package com.fasterxml.jackson.jaxrs.cbor;
 
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
@@ -17,6 +18,9 @@
 public class CBORMapperConfigurator
     extends MapperConfiguratorBase<CBORMapperConfigurator, ObjectMapper>
 {
+    // @since 2.17.1
+    private final ReentrantLock _lock = new ReentrantLock();
+
     /*
     /**********************************************************
     /* Construction
@@ -32,18 +36,24 @@ public CBORMapperConfigurator(ObjectMapper mapper, Annotations[] defAnnotations)
      * Method that locates, configures and returns {@link ObjectMapper} to use
      */
     @Override
-    public synchronized ObjectMapper getConfiguredMapper() {
-        /* important: should NOT call mapper(); needs to return null
-         * if no instance has been passed or constructed
-         */
+    public ObjectMapper getConfiguredMapper() {
+        // important: should NOT call mapper(); needs to return null
+        // if no instance has been passed or constructed
         return _mapper;
     }
 
     @Override
-    public synchronized ObjectMapper getDefaultMapper() {
+    public ObjectMapper getDefaultMapper() {
         if (_defaultMapper == null) {
-            _defaultMapper = new ObjectMapper(new CBORFactory());
-            _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_defaultMapper == null) {
+                    _defaultMapper = new ObjectMapper(new CBORFactory());
+                    _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _defaultMapper;
     }
@@ -63,8 +73,15 @@ public synchronized ObjectMapper getDefaultMapper() {
     protected ObjectMapper mapper()
     {
         if (_mapper == null) {
-            _mapper = new ObjectMapper(new CBORFactory());
-            _setAnnotations(_mapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_mapper == null) {
+                    _mapper = new ObjectMapper(new CBORFactory());
+                    _setAnnotations(_mapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _mapper;
     }

diff --git a/json/src/main/java/com/fasterxml/jackson/jaxrs/json/JsonMapperConfigurator.java b/json/src/main/java/com/fasterxml/jackson/jaxrs/json/JsonMapperConfigurator.java
@@ -1,6 +1,7 @@
 package com.fasterxml.jackson.jaxrs.json;
 
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
@@ -16,6 +17,9 @@
 public class JsonMapperConfigurator
     extends MapperConfiguratorBase<JsonMapperConfigurator, ObjectMapper>
 {
+    // @since 2.17.1
+    private final ReentrantLock _lock = new ReentrantLock();
+
     /*
     /**********************************************************
     /* Construction
@@ -31,18 +35,24 @@ public JsonMapperConfigurator(ObjectMapper mapper, Annotations[] defAnnotations)
      * Method that locates, configures and returns {@link ObjectMapper} to use
      */
     @Override
-    public synchronized ObjectMapper getConfiguredMapper() {
-        /* important: should NOT call mapper(); needs to return null
-         * if no instance has been passed or constructed
-         */
+    public ObjectMapper getConfiguredMapper() {
+        // important: should NOT call mapper(); needs to return null
+        // if no instance has been passed or constructed
         return _mapper;
     }
 
     @Override
-    public synchronized ObjectMapper getDefaultMapper() {
+    public ObjectMapper getDefaultMapper() {
         if (_defaultMapper == null) {
-            _defaultMapper = new ObjectMapper();
-            _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_defaultMapper == null) {
+                    _defaultMapper = new ObjectMapper();
+                    _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _defaultMapper;
     }
@@ -62,8 +72,15 @@ public synchronized ObjectMapper getDefaultMapper() {
     protected ObjectMapper mapper()
     {
         if (_mapper == null) {
-            _mapper = new ObjectMapper();
-            _setAnnotations(_mapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_mapper == null) {
+                    _mapper = new ObjectMapper();
+                    _setAnnotations(_mapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _mapper;
     }

diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -91,6 +91,8 @@ PJ Fanning (@pjfanning)
 * Contributed #166: `ProviderBase` class shows contention on synchronized
   block using `LRUMap` _writers instance
  (2.14.2)
+* Contributed #184: Use `ReentrantLock`s instead of synchronized blocks
+ (2.18.0)
 
 Steven Schlansker (@stevenschlansker)
 

diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -10,6 +10,11 @@ Sub-modules:
 === Releases ===
 ------------------------------------------------------------------------
 
+2.17.1 (not yet released)
+
+#184: Use `ReentrantLock`s instead of synchronized blocks
+ (contributed by @pjfanning)
+
 2.17.0 (12-Mar-2024)
 
 * Woodstox dependency now 6.6.1

diff --git a/smile/src/main/java/com/fasterxml/jackson/jaxrs/smile/SmileMapperConfigurator.java b/smile/src/main/java/com/fasterxml/jackson/jaxrs/smile/SmileMapperConfigurator.java
@@ -1,6 +1,7 @@
 package com.fasterxml.jackson.jaxrs.smile;
 
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
@@ -18,6 +19,9 @@
 public class SmileMapperConfigurator
     extends MapperConfiguratorBase<SmileMapperConfigurator, ObjectMapper>
 {
+    // @since 2.17.1
+    private final ReentrantLock _lock = new ReentrantLock();
+
     /*
     /**********************************************************
     /* Construction
@@ -33,18 +37,24 @@ public SmileMapperConfigurator(ObjectMapper mapper, Annotations[] defAnnotations
      * Method that locates, configures and returns {@link ObjectMapper} to use
      */
     @Override
-    public synchronized ObjectMapper getConfiguredMapper() {
-        /* important: should NOT call mapper(); needs to return null
-         * if no instance has been passed or constructed
-         */
+    public ObjectMapper getConfiguredMapper() {
+        // important: should NOT call mapper(); needs to return null
+        // if no instance has been passed or constructed
         return _mapper;
     }
 
     @Override
-    public synchronized ObjectMapper getDefaultMapper() {
+    public ObjectMapper getDefaultMapper() {
         if (_defaultMapper == null) {
-            _defaultMapper = new ObjectMapper(new SmileFactory());
-            _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_defaultMapper == null) {
+                    _defaultMapper = new ObjectMapper(new SmileFactory());
+                    _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _defaultMapper;
     }
@@ -64,8 +74,15 @@ public synchronized ObjectMapper getDefaultMapper() {
     protected ObjectMapper mapper()
     {
         if (_mapper == null) {
-            _mapper = new ObjectMapper(new SmileFactory());
-            _setAnnotations(_mapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_mapper == null) {
+                    _mapper = new ObjectMapper(new SmileFactory());
+                    _setAnnotations(_mapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _mapper;
     }

diff --git a/xml/src/main/java/com/fasterxml/jackson/jaxrs/xml/XMLMapperConfigurator.java b/xml/src/main/java/com/fasterxml/jackson/jaxrs/xml/XMLMapperConfigurator.java
@@ -1,6 +1,7 @@
 package com.fasterxml.jackson.jaxrs.xml;
 
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import com.fasterxml.jackson.databind.*;
 
@@ -20,6 +21,9 @@
 public class XMLMapperConfigurator
     extends MapperConfiguratorBase<XMLMapperConfigurator, XmlMapper>
 {
+    // @since 2.17.1
+    private final ReentrantLock _lock = new ReentrantLock();
+
     /*
     /**********************************************************
     /* Construction
@@ -35,21 +39,27 @@ public XMLMapperConfigurator(XmlMapper mapper, Annotations[] defAnnotations)
      * Method that locates, configures and returns {@link XmlMapper} to use
      */
     @Override
-    public synchronized XmlMapper getConfiguredMapper() {
-        /* important: should NOT call mapper(); needs to return null
-         * if no instance has been passed or constructed
-         */
+    public XmlMapper getConfiguredMapper() {
+        // important: should NOT call mapper(); needs to return null
+        // if no instance has been passed or constructed
         return _mapper;
     }
 
     @Override
-    public synchronized XmlMapper getDefaultMapper()
+    public XmlMapper getDefaultMapper()
     {
         if (_defaultMapper == null) {
-            // 10-Oct-2012, tatu: Better do things explicitly...
-            JacksonXmlModule module = getConfiguredModule();
-            _defaultMapper = (module == null) ? new XmlMapper() : new XmlMapper(module);
-            _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_defaultMapper == null) {
+                    // 10-Oct-2012, tatu: Better do things explicitly...
+                    JacksonXmlModule module = getConfiguredModule();
+                    _defaultMapper = (module == null) ? new XmlMapper() : new XmlMapper(module);
+                    _setAnnotations(_defaultMapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _defaultMapper;
     }
@@ -74,8 +84,15 @@ protected JacksonXmlModule getConfiguredModule()
     protected XmlMapper mapper()
     {
         if (_mapper == null) {
-            _mapper = new XmlMapper();
-            _setAnnotations(_mapper, _defaultAnnotationsToUse);
+            _lock.lock();
+            try {
+                if (_mapper == null) {
+                    _mapper = new XmlMapper();
+                    _setAnnotations(_mapper, _defaultAnnotationsToUse);
+                }
+            } finally {
+                _lock.unlock();
+            }
         }
         return _mapper;
     }

diff --git a/yaml/src/main/java/com/fasterxml/jackson/jaxrs/yaml/JacksonYAMLProvider.java b/yaml/src/main/java/com/fasterxml/jackson/jaxrs/yaml/JacksonYAMLProvider.java
@@ -29,7 +29,7 @@
  * mapper to use can be configured in multiple ways:
  * <ul>
  * <li>By explicitly passing mapper to use in constructor
- * <li>By explcitly setting mapper to use by {@link #setMapper}
+ * <li>By explicitly setting mapper to use by {@link #setMapper}
  * <li>By defining JAX-RS <code>Provider</code> that returns {@link YAMLMapper}s.
  * <li>By doing none of above, in which case a default mapper instance is
  * constructed (and configured if configuration methods are called)