From 651fa8c9eaf392aa44a3e48866deacd715a5040e Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Thu, 28 Mar 2024 17:24:16 +0100 Subject: [PATCH 1/5] remove synchronized block --- .../databind/deser/DeserializerCache.java | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java index 28127c1af0..0a8a11cb6e 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java @@ -53,7 +53,6 @@ public final class DeserializerCache protected final HashMap> _incompleteDeserializers = new HashMap>(8); - /** * We hold an explicit lock while creating deserializers to avoid creating duplicates. */ @@ -253,14 +252,26 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat * limitations necessary to ensure that only completely initialized ones * are visible and used. */ + if (type == null) { + throw new IllegalArgumentException("Null JavaType passed"); + } + if (_hasCustomHandlers(type)) { + return null; + } + final boolean isCustom = _hasCustomHandlers(type); + JsonDeserializer deser = isCustom ? null : _cachedDeserializers.get(type); + if (deser != null) { + return deser; + } + _incompleteDeserializersLock.lock(); try { - _incompleteDeserializersLock.lock(); - - // Ok, then: could it be that due to a race condition, deserializer can now be found? - JsonDeserializer deser = _findCachedDeserializer(type); - if (deser != null) { - return deser; + if (!isCustom) { + deser = _cachedDeserializers.get(type); + if (deser != null) { + return deser; + } } + // Ok, then: could it be that due to a race condition, deserializer can now be found? int count = _incompleteDeserializers.size(); // Or perhaps being resolved right now? if (count > 0) { From f2115286e88a0c3b05ef60a11e2a81be192f0d47 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Thu, 28 Mar 2024 17:29:39 +0100 Subject: [PATCH 2/5] Update DeserializerCache.java --- .../fasterxml/jackson/databind/deser/DeserializerCache.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java index 0a8a11cb6e..c06e273b74 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java @@ -255,9 +255,6 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat if (type == null) { throw new IllegalArgumentException("Null JavaType passed"); } - if (_hasCustomHandlers(type)) { - return null; - } final boolean isCustom = _hasCustomHandlers(type); JsonDeserializer deser = isCustom ? null : _cachedDeserializers.get(type); if (deser != null) { From eeef0c308dd27386eae5a60f40fe9f0f8f6c0bd8 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sat, 30 Mar 2024 00:11:53 +0100 Subject: [PATCH 3/5] Update DeserializerCache.java --- .../jackson/databind/deser/DeserializerCache.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java index c06e273b74..d2408e305c 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java @@ -279,7 +279,7 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat } // Nope: need to create and possibly cache try { - return _createAndCache2(ctxt, factory, type); + return _createAndCache2(ctxt, factory, type, isCustom); } finally { // also: any deserializers that have been created are complete by now if (count == 0 && _incompleteDeserializers.size() > 0) { @@ -294,9 +294,18 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat /** * Method that handles actual construction (via factory) and caching (both * intermediate and eventual) + * + * @deprecated use version of _createAndCache2 that takes `isCustom` flag */ protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, - DeserializerFactory factory, JavaType type) + DeserializerFactory factory, JavaType type) + throws JsonMappingException + { + return _createAndCache2(ctxt, factory, type, _hasCustomHandlers(type)); + } + + protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, + DeserializerFactory factory, JavaType type, final boolean isCustom) throws JsonMappingException { JsonDeserializer deser; @@ -315,7 +324,7 @@ protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, * (but can be re-defined for sub-classes by using @JsonCachable!) */ // 27-Mar-2015, tatu: As per [databind#735], avoid caching types with custom value desers - boolean addToCache = !_hasCustomHandlers(type) && deser.isCachable(); + boolean addToCache = !isCustom && deser.isCachable(); /* we will temporarily hold on to all created deserializers (to * handle cyclic references, and possibly reuse non-cached From 361ed2f4ee6506c354e1f4a7ce426e9f2a81d3e7 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sun, 31 Mar 2024 18:58:37 -0700 Subject: [PATCH 4/5] Add "since" notes on deprecation --- .../fasterxml/jackson/databind/deser/DeserializerCache.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java index da8c54f409..3e25c14215 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java @@ -293,9 +293,9 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat * Method that handles actual construction (via factory) and caching (both * intermediate and eventual) * - * @deprecated use version of _createAndCache2 that takes `isCustom` flag + * @deprecated Since 2.18 use version of _createAndCache2 that takes `isCustom` flag */ - @Deprecated + @Deprecated // since 2.18 protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, DeserializerFactory factory, JavaType type) throws JsonMappingException From 9a27ef4b8e5b8367f770cfef1d064e37aa4e5b3d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Sat, 6 Apr 2024 22:13:33 -0700 Subject: [PATCH 5/5] Add release notes, minor tweaking --- release-notes/VERSION-2.x | 2 ++ .../databind/deser/DeserializerCache.java | 28 ++++++++++--------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 30151bbfd9..6c1aa39a1b 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -6,6 +6,8 @@ Project: jackson-databind 2.18.0 (not yet released) +#4456: Rework locking in `DeserializerCache` + (contributed by @pjfanning) #4464: When `Include.NON_DEFAULT` setting is used, `isEmpty()` method is not called on the serializer (reported by Teodor D) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java index 3e25c14215..7a40697324 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/DeserializerCache.java @@ -254,14 +254,16 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat // limitations necessary to ensure that only completely initialized ones // are visible and used. final boolean isCustom = _hasCustomHandlers(type); - JsonDeserializer deser = isCustom ? null : _cachedDeserializers.get(type); - if (deser != null) { - return deser; + if (!isCustom) { + JsonDeserializer deser = _cachedDeserializers.get(type); + if (deser != null) { + return deser; + } } _incompleteDeserializersLock.lock(); try { if (!isCustom) { - deser = _cachedDeserializers.get(type); + JsonDeserializer deser = _cachedDeserializers.get(type); if (deser != null) { return deser; } @@ -270,7 +272,7 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat int count = _incompleteDeserializers.size(); // Or perhaps being resolved right now? if (count > 0) { - deser = _incompleteDeserializers.get(type); + JsonDeserializer deser = _incompleteDeserializers.get(type); if (deser != null) { return deser; } @@ -295,7 +297,7 @@ protected JsonDeserializer _createAndCacheValueDeserializer(Deserializat * * @deprecated Since 2.18 use version of _createAndCache2 that takes `isCustom` flag */ - @Deprecated // since 2.18 + @Deprecated // since 2.18, remove from 2.19 protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, DeserializerFactory factory, JavaType type) throws JsonMappingException @@ -303,8 +305,9 @@ protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, return _createAndCache2(ctxt, factory, type, _hasCustomHandlers(type)); } + // @since 2.18 protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, - DeserializerFactory factory, JavaType type, final boolean isCustom) + DeserializerFactory factory, JavaType type, boolean isCustom) throws JsonMappingException { JsonDeserializer deser; @@ -319,9 +322,9 @@ protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, if (deser == null) { return null; } - /* cache resulting deserializer? always true for "plain" BeanDeserializer - * (but can be re-defined for sub-classes by using @JsonCachable!) - */ + // cache resulting deserializer? always true for "plain" BeanDeserializer + // (but can be re-defined for sub-classes by using @JsonCachable!) + // 27-Mar-2015, tatu: As per [databind#735], avoid caching types with custom value desers boolean addToCache = !isCustom && deser.isCachable(); @@ -334,9 +337,8 @@ protected JsonDeserializer _createAndCache2(DeserializationContext ctxt, * either not add Lists or Maps, or clear references eagerly. * Let's actually do both; since both seem reasonable. */ - /* Need to resolve? Mostly done for bean deserializers; required for - * resolving cyclic references. - */ + // Need to resolve? Mostly done for bean deserializers; required for + // resolving cyclic references. if (deser instanceof ResolvableDeserializer) { _incompleteDeserializers.put(type, deser); try {