From 7d6ea9b833a6420cb237417c50abf034f10b54f6 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Thu, 20 Jun 2024 11:00:54 +0200 Subject: [PATCH 1/2] Use one lock per serialization / deserialization beans to prevent deadlock --- .../support/deserializers/DeserBean.java | 17 +++++--------- .../deserializers/ObjectDeserializer.java | 7 +++++- .../support/serializers/ObjectSerializer.java | 7 +++++- .../serde/support/serializers/SerBean.java | 22 ++++++++----------- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java index 994bbb29e..53bd91038 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java @@ -110,7 +110,7 @@ final class DeserBean { private final Map> bounds; - private volatile boolean initialized; + volatile boolean initialized; private volatile boolean initializing; // CHECKSTYLE:ON @@ -427,16 +427,11 @@ public AnnotationMetadata getAnnotationMetadata() { } void initialize(Deserializer.DecoderContext decoderContext) throws SerdeException { - // Double check locking - if (!initialized) { - synchronized (this) { - if (!initialized && !initializing) { - initializing = true; - initializeInternal(decoderContext); - initialized = true; - initializing = false; - } - } + if (!initialized && !initializing) { + initializing = true; + initializeInternal(decoderContext); + initialized = true; + initializing = false; } } diff --git a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java index 83d22e7cb..39ebbebc5 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java @@ -155,7 +155,12 @@ public DeserBean getDeserializableBean(Argument type, DecoderContext d // Use suppliers to prevent recursive update because the lambda can call the same method again Supplier> deserBeanSupplier = deserBeanMap.computeIfAbsent(key, ignore -> SupplierUtil.memoizedNonEmpty(() -> createDeserBean(type, serdeArgumentConf, decoderContext))); DeserBean deserBean = deserBeanSupplier.get(); - deserBean.initialize(decoderContext); + // Double check locking + if (!deserBean.initialized) { + synchronized (this) { + deserBean.initialize(decoderContext); + } + } return (DeserBean) deserBean; } diff --git a/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java b/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java index 1dec4fc06..c53b34237 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java @@ -162,7 +162,12 @@ private SerBean getSerializableBean(Argument type, } })); SerBean serBean = serBeanSupplier.get(); - serBean.initialize(context); + // Double check locking + if (!serBean.initialized) { + synchronized (this) { + serBean.initialize(context); + } + } return (SerBean) serBean; } } diff --git a/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java b/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java index e88374f56..89b14fb2d 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java @@ -97,7 +97,7 @@ public int getOrder() { @Nullable private final SerdeArgumentConf serdeArgumentConf; - private volatile boolean initialized; + volatile boolean initialized; private volatile boolean initializing; private List initializers = new ArrayList<>(); @@ -377,19 +377,15 @@ private static PropertySubtypeDescriptor findDescriptor(@Nullable SubtypeInfo su } } - public void initialize(Serializer.EncoderContext encoderContext) throws SerdeException { - if (!initialized) { - synchronized (this) { - if (!initialized && !initializing) { - initializing = true; - for (Initializer initializer : initializers) { - initializer.initialize(encoderContext); - } - initializers = null; - initialized = true; - initializing = false; - } + void initialize(Serializer.EncoderContext encoderContext) throws SerdeException { + if (!initialized && !initializing) { + initializing = true; + for (Initializer initializer : initializers) { + initializer.initialize(encoderContext); } + initializers = null; + initialized = true; + initializing = false; } } From 0a5b8b3dedbb6d380032d4a54c5752a0ddb50d47 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Thu, 20 Jun 2024 11:23:08 +0200 Subject: [PATCH 2/2] use `ReentrantLock` --- .../support/deserializers/DeserBean.java | 23 +++++++++++----- .../deserializers/ObjectDeserializer.java | 10 +++---- .../support/serializers/ObjectSerializer.java | 10 +++---- .../serde/support/serializers/SerBean.java | 27 ++++++++++++------- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java index 53bd91038..f0d43795d 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/DeserBean.java @@ -61,6 +61,7 @@ import java.util.OptionalInt; import java.util.OptionalLong; import java.util.Set; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiConsumer; import java.util.function.Predicate; @@ -110,7 +111,7 @@ final class DeserBean { private final Map> bounds; - volatile boolean initialized; + private volatile boolean initialized; private volatile boolean initializing; // CHECKSTYLE:ON @@ -426,12 +427,20 @@ public AnnotationMetadata getAnnotationMetadata() { recordLikeBean = isRecordLikeBean(); } - void initialize(Deserializer.DecoderContext decoderContext) throws SerdeException { - if (!initialized && !initializing) { - initializing = true; - initializeInternal(decoderContext); - initialized = true; - initializing = false; + void initialize(ReentrantLock lock, Deserializer.DecoderContext decoderContext) throws SerdeException { + // Double check locking + if (!initialized) { + lock.lock(); + try { + if (!initialized && !initializing) { + initializing = true; + initializeInternal(decoderContext); + initialized = true; + initializing = false; + } + } finally { + lock.unlock(); + } } } diff --git a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java index 39ebbebc5..7f6545cce 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/deserializers/ObjectDeserializer.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; /** @@ -51,6 +52,8 @@ public class ObjectDeserializer implements CustomizableDeserializer, Des @Nullable private final SerdeDeserializationPreInstantiateCallback preInstantiateCallback; + private final ReentrantLock lock = new ReentrantLock(); + public ObjectDeserializer(SerdeIntrospections introspections, DeserializationConfiguration deserializationConfiguration, SerdeConfiguration serdeConfiguration, @@ -155,12 +158,7 @@ public DeserBean getDeserializableBean(Argument type, DecoderContext d // Use suppliers to prevent recursive update because the lambda can call the same method again Supplier> deserBeanSupplier = deserBeanMap.computeIfAbsent(key, ignore -> SupplierUtil.memoizedNonEmpty(() -> createDeserBean(type, serdeArgumentConf, decoderContext))); DeserBean deserBean = deserBeanSupplier.get(); - // Double check locking - if (!deserBean.initialized) { - synchronized (this) { - deserBean.initialize(decoderContext); - } - } + deserBean.initialize(lock, decoderContext); return (DeserBean) deserBean; } diff --git a/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java b/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java index c53b34237..af6c3b599 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/serializers/ObjectSerializer.java @@ -36,6 +36,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; /** @@ -59,6 +60,8 @@ public final class ObjectSerializer implements CustomizableSerializer { @Nullable private final BeanContext beanContext; + private final ReentrantLock lock = new ReentrantLock(); + public ObjectSerializer(SerdeIntrospections introspections, SerdeConfiguration serdeConfiguration, SerializationConfiguration serializationConfiguration) { @@ -162,12 +165,7 @@ private SerBean getSerializableBean(Argument type, } })); SerBean serBean = serBeanSupplier.get(); - // Double check locking - if (!serBean.initialized) { - synchronized (this) { - serBean.initialize(context); - } - } + serBean.initialize(lock, context); return (SerBean) serBean; } } diff --git a/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java b/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java index 89b14fb2d..149917574 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/serializers/SerBean.java @@ -58,6 +58,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; import java.util.function.Predicate; @@ -97,7 +98,7 @@ public int getOrder() { @Nullable private final SerdeArgumentConf serdeArgumentConf; - volatile boolean initialized; + private volatile boolean initialized; private volatile boolean initializing; private List initializers = new ArrayList<>(); @@ -377,15 +378,23 @@ private static PropertySubtypeDescriptor findDescriptor(@Nullable SubtypeInfo su } } - void initialize(Serializer.EncoderContext encoderContext) throws SerdeException { - if (!initialized && !initializing) { - initializing = true; - for (Initializer initializer : initializers) { - initializer.initialize(encoderContext); + public void initialize(ReentrantLock lock, Serializer.EncoderContext encoderContext) throws SerdeException { + // Double check locking + if (!initialized) { + lock.lock(); + try { + if (!initialized && !initializing) { + initializing = true; + for (Initializer initializer : initializers) { + initializer.initialize(encoderContext); + } + initializers = null; + initialized = true; + initializing = false; + } + } finally { + lock.unlock(); } - initializers = null; - initialized = true; - initializing = false; } }