From f0076956b1b2ecc6bc1ea83dbee95f4dc70a5c66 Mon Sep 17 00:00:00 2001 From: Milenko Supic Date: Fri, 12 May 2023 12:05:44 +0200 Subject: [PATCH 1/5] Fixed concurrency issue in ObjectDeserializer --- .../deserializers/ObjectDeserializer.java | 2 +- .../serde/support/CoreTypeSerdeSpec.groovy | 29 +++++++++++++++++++ .../micronaut/serde/support/SimpleInfo.java | 16 ++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 serde-support/src/test/java/io/micronaut/serde/support/SimpleInfo.java 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 31c756788..08e60d616 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 @@ -78,8 +78,8 @@ public DeserBean getDeserializableBean(Argument type, DecoderContext d DeserBean deserBeanSupplier = (DeserBean) deserBeanMap.get(key); if (deserBeanSupplier == null) { deserBeanSupplier = createDeserBean(type, decoderContext); - deserBeanMap.put(key, (DeserBean) deserBeanSupplier); deserBeanSupplier.initialize(decoderContext); + deserBeanMap.put(key, (DeserBean) deserBeanSupplier); } return deserBeanSupplier; } diff --git a/serde-support/src/test/groovy/io/micronaut/serde/support/CoreTypeSerdeSpec.groovy b/serde-support/src/test/groovy/io/micronaut/serde/support/CoreTypeSerdeSpec.groovy index a820341c5..b7e624b29 100644 --- a/serde-support/src/test/groovy/io/micronaut/serde/support/CoreTypeSerdeSpec.groovy +++ b/serde-support/src/test/groovy/io/micronaut/serde/support/CoreTypeSerdeSpec.groovy @@ -12,6 +12,11 @@ import jakarta.inject.Inject import spock.lang.Specification import java.nio.charset.StandardCharsets +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.stream.IntStream @MicronautTest class CoreTypeSerdeSpec extends Specification { @@ -86,4 +91,28 @@ class CoreTypeSerdeSpec extends Specification { then: read == node } + + void "test read concurrency"() { + given: + def results = new ConcurrentHashMap() + + when: + ExecutorService threadPool = Executors.newFixedThreadPool(1000) + IntStream.range(0, 1000).forEach(i -> threadPool.execute(new Runnable() { + @Override + void run() { + results.put(i, jsonMapper.readValue('{"info":"test' + i + '"}', Argument.of(SimpleInfo))) + } + })); + threadPool.shutdown() + try { + threadPool.awaitTermination(1, TimeUnit.MINUTES) + } catch (InterruptedException e) { + throw new RuntimeException("Interrupted while waiting", e) + } + + then: + results.size() == 1000 + results.each { assert it.value.getInfo() == "test" + it.key } + } } diff --git a/serde-support/src/test/java/io/micronaut/serde/support/SimpleInfo.java b/serde-support/src/test/java/io/micronaut/serde/support/SimpleInfo.java new file mode 100644 index 000000000..1d22f52cc --- /dev/null +++ b/serde-support/src/test/java/io/micronaut/serde/support/SimpleInfo.java @@ -0,0 +1,16 @@ +package io.micronaut.serde.support; + +import io.micronaut.serde.annotation.Serdeable; + +@Serdeable +public class SimpleInfo { + private final String info; + + public SimpleInfo(String info) { + this.info = info; + } + + public String getInfo() { + return info; + } +} From b12689a383d2b218805ee1cf8e97f552cca5bbb9 Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Mon, 15 May 2023 07:52:35 -0500 Subject: [PATCH 2/5] Correct deserialier map --- .../deserializers/ObjectDeserializer.java | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) 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 08e60d616..92eb8ebef 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 @@ -21,6 +21,7 @@ import io.micronaut.core.beans.BeanIntrospection; import io.micronaut.core.beans.exceptions.IntrospectionException; import io.micronaut.core.type.Argument; +import io.micronaut.core.util.SupplierUtil; import io.micronaut.inject.annotation.AnnotationMetadataHierarchy; import io.micronaut.serde.Decoder; import io.micronaut.serde.Deserializer; @@ -34,6 +35,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; /** * Implementation for deserialization of objects that uses introspection metadata. @@ -47,7 +49,7 @@ public class ObjectDeserializer implements CustomizableDeserializer, DeserBeanRegistry { private final SerdeIntrospections introspections; private final boolean ignoreUnknown; - private final Map> deserBeanMap = new ConcurrentHashMap<>(50); + private final Map>> deserBeanMap = new ConcurrentHashMap<>(50); public ObjectDeserializer(SerdeIntrospections introspections, DeserializationConfiguration deserializationConfiguration) { this.introspections = introspections; @@ -75,13 +77,27 @@ public Deserializer createSpecific(DecoderContext context, Argument DeserBean getDeserializableBean(Argument type, DecoderContext decoderContext) throws SerdeException { TypeKey key = new TypeKey(type); - DeserBean deserBeanSupplier = (DeserBean) deserBeanMap.get(key); + Supplier> deserBeanSupplier = deserBeanMap.get(key); if (deserBeanSupplier == null) { - deserBeanSupplier = createDeserBean(type, decoderContext); - deserBeanSupplier.initialize(decoderContext); - deserBeanMap.put(key, (DeserBean) deserBeanSupplier); + try { + deserBeanSupplier = SupplierUtil.memoizedNonEmpty(() -> { + DeserBean deserBean = createDeserBean(type, decoderContext); + try { + deserBean.initialize(decoderContext); + } catch (SerdeException e) { + throw new RuntimeException(e); + } + return deserBean; + }); + deserBeanMap.put(key, deserBeanSupplier); + } catch (RuntimeException e) { + if (e.getCause() instanceof SerdeException serdeException) { + throw serdeException; + } + throw e; + } } - return deserBeanSupplier; + return (DeserBean) deserBeanSupplier.get(); } private DeserBean createDeserBean(Argument type, DecoderContext decoderContext) { @@ -94,7 +110,7 @@ private DeserBean createDeserBean(Argument type, DecoderContext decode if (annotationMetadata.hasAnnotation(SerdeConfig.SerSubtyped.class)) { if (type.hasTypeVariables()) { final Map> bounds = type.getTypeVariables(); - return new SubtypedDeserBean(annotationMetadata, deserializableIntrospection, decoderContext, this) { + return new SubtypedDeserBean<>(annotationMetadata, deserializableIntrospection, decoderContext, this) { @Override protected Map> getBounds() { return bounds; @@ -106,7 +122,7 @@ protected Map> getBounds() { } else { if (type.hasTypeVariables()) { final Map> bounds = type.getTypeVariables(); - return new DeserBean(deserializableIntrospection, decoderContext, this) { + return new DeserBean<>(deserializableIntrospection, decoderContext, this) { @Override protected Map> getBounds() { return bounds; From 090ac063cff5f1814f02eab4a2c9ae44838dd42c Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Mon, 15 May 2023 08:57:58 -0500 Subject: [PATCH 3/5] Use double check locking --- .../support/deserializers/DeserBean.java | 14 +++++++++ .../deserializers/ObjectDeserializer.java | 30 +++++-------------- 2 files changed, 21 insertions(+), 23 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 3f0a68a02..e98ce5295 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 @@ -86,6 +86,8 @@ class DeserBean { public final boolean recordLikeBean; public final ConversionService conversionService; + private volatile boolean initialized; + // CHECKSTYLE:ON public DeserBean( @@ -346,6 +348,18 @@ public boolean isSubtyped() { } void initialize(Deserializer.DecoderContext decoderContext) throws SerdeException { + // Double check locking + if (!initialized) { + synchronized (this) { + if (!initialized) { + initializeInternal(decoderContext); + initialized = true; + } + } + } + } + + private void initializeInternal(Deserializer.DecoderContext decoderContext) throws SerdeException { if (readProperties != null) { List>> properties = readProperties.getProperties(); for (Map.Entry> e : properties) { 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 92eb8ebef..e3aafb665 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 @@ -21,7 +21,6 @@ import io.micronaut.core.beans.BeanIntrospection; import io.micronaut.core.beans.exceptions.IntrospectionException; import io.micronaut.core.type.Argument; -import io.micronaut.core.util.SupplierUtil; import io.micronaut.inject.annotation.AnnotationMetadataHierarchy; import io.micronaut.serde.Decoder; import io.micronaut.serde.Deserializer; @@ -35,7 +34,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Supplier; /** * Implementation for deserialization of objects that uses introspection metadata. @@ -49,7 +47,7 @@ public class ObjectDeserializer implements CustomizableDeserializer, DeserBeanRegistry { private final SerdeIntrospections introspections; private final boolean ignoreUnknown; - private final Map>> deserBeanMap = new ConcurrentHashMap<>(50); + private final Map> deserBeanMap = new ConcurrentHashMap<>(50); public ObjectDeserializer(SerdeIntrospections introspections, DeserializationConfiguration deserializationConfiguration) { this.introspections = introspections; @@ -77,27 +75,13 @@ public Deserializer createSpecific(DecoderContext context, Argument DeserBean getDeserializableBean(Argument type, DecoderContext decoderContext) throws SerdeException { TypeKey key = new TypeKey(type); - Supplier> deserBeanSupplier = deserBeanMap.get(key); - if (deserBeanSupplier == null) { - try { - deserBeanSupplier = SupplierUtil.memoizedNonEmpty(() -> { - DeserBean deserBean = createDeserBean(type, decoderContext); - try { - deserBean.initialize(decoderContext); - } catch (SerdeException e) { - throw new RuntimeException(e); - } - return deserBean; - }); - deserBeanMap.put(key, deserBeanSupplier); - } catch (RuntimeException e) { - if (e.getCause() instanceof SerdeException serdeException) { - throw serdeException; - } - throw e; - } + DeserBean deserBean = (DeserBean) deserBeanMap.get(key); + if (deserBean == null) { + deserBean = createDeserBean(type, decoderContext); + deserBeanMap.put(key, deserBean); } - return (DeserBean) deserBeanSupplier.get(); + deserBean.initialize(decoderContext); + return deserBean; } private DeserBean createDeserBean(Argument type, DecoderContext decoderContext) { From a50caf2452a412702b15c009a153d3077eff188b Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Mon, 15 May 2023 09:06:38 -0500 Subject: [PATCH 4/5] Prevent reentering --- .../io/micronaut/serde/support/deserializers/DeserBean.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 e98ce5295..86da21371 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 @@ -87,6 +87,7 @@ class DeserBean { public final ConversionService conversionService; private volatile boolean initialized; + private volatile boolean initializing; // CHECKSTYLE:ON @@ -351,9 +352,11 @@ void initialize(Deserializer.DecoderContext decoderContext) throws SerdeExceptio // Double check locking if (!initialized) { synchronized (this) { - if (!initialized) { + if (!initialized && !initializing) { + initializing = true; initializeInternal(decoderContext); initialized = true; + initializing = false; } } } From 0f97e649bcba287ae6bf6495b8cfe9c7a4840c9f Mon Sep 17 00:00:00 2001 From: Denis Stepanov Date: Mon, 15 May 2023 09:08:43 -0500 Subject: [PATCH 5/5] Code style --- .../deserializers/ObjectDeserializer.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) 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 e3aafb665..c628d36b0 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 @@ -59,17 +59,15 @@ public Deserializer createSpecific(DecoderContext context, Argument type1) -> decoder.decodeArbitrary(); - } else { - DeserBean deserBean = getDeserializableBean(type, context); - if (deserBean.simpleBean) { - return new SimpleObjectDeserializer(ignoreUnknown, deserBean); - } - if (deserBean.recordLikeBean) { - return new SimpleRecordLikeObjectDeserializer(ignoreUnknown, deserBean); - } - return new SpecificObjectDeserializer(ignoreUnknown, deserBean); - } + DeserBean deserBean = getDeserializableBean(type, context); + if (deserBean.simpleBean) { + return new SimpleObjectDeserializer(ignoreUnknown, deserBean); + } + if (deserBean.recordLikeBean) { + return new SimpleRecordLikeObjectDeserializer(ignoreUnknown, deserBean); + } + return new SpecificObjectDeserializer(ignoreUnknown, deserBean); } @Override