From 9a18f0430e0d7e0e566903537b5b794a287bc6b9 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 1 Apr 2024 02:56:45 +0200 Subject: [PATCH] Use `ReentrantLock`s instead of synchronized blocks (#184) --- .../jaxrs/cfg/MapperConfiguratorBase.java | 12 +++--- .../jaxrs/cbor/CBORMapperConfigurator.java | 35 ++++++++++++----- .../jaxrs/json/JsonMapperConfigurator.java | 35 ++++++++++++----- release-notes/CREDITS-2.x | 2 + release-notes/VERSION-2.x | 5 +++ .../jaxrs/smile/SmileMapperConfigurator.java | 35 ++++++++++++----- .../jaxrs/xml/XMLMapperConfigurator.java | 39 +++++++++++++------ .../jaxrs/yaml/JacksonYAMLProvider.java | 2 +- .../jaxrs/yaml/YAMLMapperConfigurator.java | 38 +++++++++++++----- 9 files changed, 148 insertions(+), 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 index 234c9720..ad3dc06a 100644 --- 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 index 9dfda90b..6c41863b 100644 --- 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 { + // @since 2.18 + 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 index 5b22e8b3..189753ca 100644 --- 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 { + // @since 2.18 + 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 index e697345d..b0168b67 100644 --- 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 index 851638c4..128964c1 100644 --- 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 index b9d98646..1cf41c46 100644 --- 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 { + // @since 2.18 + 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 index ea875704..dc0762c9 100644 --- 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 { + // @since 2.18 + 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 index b574709a..37df7f34 100644 --- 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: *
    *
  • By explicitly passing mapper to use in constructor - *
  • By explcitly setting mapper to use by {@link #setMapper} + *
  • By explicitly setting mapper to use by {@link #setMapper} *
  • By defining JAX-RS Provider that returns {@link YAMLMapper}s. *
  • By doing none of above, in which case a default mapper instance is * constructed (and configured if configuration methods are called) diff --git a/yaml/src/main/java/com/fasterxml/jackson/jaxrs/yaml/YAMLMapperConfigurator.java b/yaml/src/main/java/com/fasterxml/jackson/jaxrs/yaml/YAMLMapperConfigurator.java index d0f99ebb..bfd73bee 100644 --- a/yaml/src/main/java/com/fasterxml/jackson/jaxrs/yaml/YAMLMapperConfigurator.java +++ b/yaml/src/main/java/com/fasterxml/jackson/jaxrs/yaml/YAMLMapperConfigurator.java @@ -8,6 +8,7 @@ import com.fasterxml.jackson.module.jaxb.JaxbAnnotationIntrospector; import java.util.ArrayList; +import java.util.concurrent.locks.ReentrantLock; /** * Helper class used to encapsulate details of configuring an @@ -15,7 +16,11 @@ * well as accessing it. */ public class YAMLMapperConfigurator - extends MapperConfiguratorBase { + extends MapperConfiguratorBase +{ + // @since 2.18 + private final ReentrantLock _lock = new ReentrantLock(); + /* /********************************************************** /* Construction @@ -30,18 +35,24 @@ public YAMLMapperConfigurator(YAMLMapper mapper, Annotations[] defAnnotations) { * Method that locates, configures and returns {@link YAMLMapper} to use */ @Override - public synchronized YAMLMapper getConfiguredMapper() { - /* important: should NOT call mapper(); needs to return null - * if no instance has been passed or constructed - */ + public YAMLMapper getConfiguredMapper() { + // important: should NOT call mapper(); needs to return null + // if no instance has been passed or constructed return _mapper; } @Override - public synchronized YAMLMapper getDefaultMapper() { + public YAMLMapper getDefaultMapper() { if (_defaultMapper == null) { - _defaultMapper = new YAMLMapper(); //tarik: maybe there is better default config? - _setAnnotations(_defaultMapper, _defaultAnnotationsToUse); + _lock.lock(); + try { + if (_defaultMapper == null) { + _defaultMapper = new YAMLMapper(); //tarik: maybe there is better default config? + _setAnnotations(_defaultMapper, _defaultAnnotationsToUse); + } + } finally { + _lock.unlock(); + } } return _defaultMapper; } @@ -60,8 +71,15 @@ public synchronized YAMLMapper getDefaultMapper() { @Override protected YAMLMapper mapper() { if (_mapper == null) { - _mapper = new YAMLMapper(); - _setAnnotations(_mapper, _defaultAnnotationsToUse); + _lock.lock(); + try { + if (_mapper == null) { + _mapper = new YAMLMapper(); + _setAnnotations(_mapper, _defaultAnnotationsToUse); + } + } finally { + _lock.unlock(); + } } return _mapper; }