You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bval.apache.org by rm...@apache.org on 2019/04/19 09:01:42 UTC

[bval] branch master updated: BVAL-172 more thread safety in our caches

This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bval.git


The following commit(s) were added to refs/heads/master by this push:
     new c4e1bdf  BVAL-172 more thread safety in our caches
c4e1bdf is described below

commit c4e1bdf29657af63749c8e0af73aea3485774097
Author: Romain Manni-Bucau <rm...@gmail.com>
AuthorDate: Fri Apr 19 11:01:28 2019 +0200

    BVAL-172 more thread safety in our caches
---
 bval-jsr/pom.xml                                   | 27 ++++++++++++++++++++++
 .../java/org/apache/bval/jsr/ConstraintCached.java |  6 ++---
 .../bval/jsr/descriptor/CascadableContainerD.java  | 12 ++++++----
 .../apache/bval/jsr/metadata/HierarchyBuilder.java |  8 ++++---
 .../bval/jsr/util/AnnotationProxyBuilder.java      |  4 +++-
 .../java/org/apache/bval/jsr/util/LRUCache.java    |  2 +-
 6 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/bval-jsr/pom.xml b/bval-jsr/pom.xml
index 1405487..b74bf1b 100644
--- a/bval-jsr/pom.xml
+++ b/bval-jsr/pom.xml
@@ -37,6 +37,33 @@
         <jaxb.version>2.2.6</jaxb.version>
     </properties>
     <profiles>
+
+        <profile>
+            <id>java11</id>
+            <activation>
+                <jdk>11</jdk>
+            </activation>
+            <dependencies>
+                <dependency>
+                    <groupId>javax.xml.bind</groupId>
+                    <artifactId>jaxb-api</artifactId>
+                    <version>2.3.1</version>
+                    <scope>provided</scope>
+                </dependency>
+                <dependency>
+                    <groupId>com.sun.xml.bind</groupId>
+                    <artifactId>jaxb-core</artifactId>
+                    <version>2.3.1</version>
+                    <scope>provided</scope>
+                </dependency>
+                <dependency>
+                    <groupId>org.openjfx</groupId>
+                    <artifactId>javafx-base</artifactId>
+                    <version>11</version>
+                    <scope>provided</scope>
+                </dependency>
+            </dependencies>
+        </profile>
         <!--
             default profile using geronimo-validation_2.0_spec.jar active when
             property "ri" is not present.
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java
index 9622b9e..7af0488 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java
@@ -23,9 +23,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -99,8 +97,8 @@ public class ConstraintCached {
         }
     }
 
-    private final Map<Class<? extends Annotation>, Set<ConstraintValidatorInfo<?>>> constraintValidatorInfo =
-        new HashMap<>();
+    private final ConcurrentMap<Class<? extends Annotation>, Set<ConstraintValidatorInfo<?>>> constraintValidatorInfo =
+        new ConcurrentHashMap<>();
     private final ConcurrentMap<ConstraintD<?>, ConstraintValidator<?, ?>> validators =
         new ConcurrentHashMap<>();
 
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java
index 117e870..ebb3c33 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java
@@ -39,10 +39,14 @@ public abstract class CascadableContainerD<P extends ElementD<?, ?>, E extends A
 
     protected CascadableContainerD(MetadataReader.ForContainer<E> reader, P parent) {
         super(reader, parent);
-        cascaded = reader.isCascaded();
-        groupConversions = reader.getGroupConversions();
-        containerElementTypes = reader.getContainerElementTypes(this).stream().filter(DescriptorManager::isConstrained)
-                .collect(ToUnmodifiable.set());
+        synchronized (reader.meta.getHost()) { // cache is not thread safe for runtime perf so ensure we lock properly
+            cascaded = reader.isCascaded();
+            groupConversions = reader.getGroupConversions();
+            containerElementTypes = reader.getContainerElementTypes(this)
+                                          .stream()
+                                          .filter(DescriptorManager::isConstrained)
+                                          .collect(ToUnmodifiable.set());
+        }
     }
 
     @Override
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java
index da84490..e9209fb 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/HierarchyBuilder.java
@@ -334,8 +334,10 @@ public class HierarchyBuilder extends CompositeBuilder {
             return (MetadataBuilder.ForBean<T>) delegates.get(0);
         }
         // pretend:
-        return delegates.stream().<MetadataBuilder.ForBean<T>> map(MetadataBuilder.ForBean.class::cast)
-            .collect(compose());
+        // note: stream split for java 11 compilation
+        final Stream<MetadataBuilder.ForBean<T>> forBeanStream = delegates.stream()
+                .map(MetadataBuilder.ForBean.class::cast);
+        return forBeanStream.collect(compose());
     }
 
     @Override
@@ -345,7 +347,7 @@ public class HierarchyBuilder extends CompositeBuilder {
         @SuppressWarnings("unchecked")
         final Function<MetadataBuilder.ForElement<E>, Meta<E>> keyMapper =
             d -> Optional.of(d).filter(HierarchyDelegate.class::isInstance).map(HierarchyDelegate.class::cast)
-                .map(HierarchyDelegate::getHierarchyElement).orElse(meta);
+                .map(HierarchyDelegate::getHierarchyElement).map(Meta.class::cast).orElse(meta);
 
         return composite.delegates.stream().collect(Collectors.toMap(keyMapper, d -> d.getDeclaredConstraints(meta),
             (u, v) -> Stream.of(u, v).flatMap(Stream::of).toArray(Annotation[]::new), LinkedHashMap::new));
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java
index ee9a139..cfa4c48 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java
@@ -59,7 +59,9 @@ public final class AnnotationProxyBuilder<A extends Annotation> {
      */
     AnnotationProxyBuilder(final Class<A> annotationType, Map<Class<?>, Method[]> cache) {
         this.type = Validate.notNull(annotationType, "annotationType");
-        this.methods = Validate.notNull(cache, "cache").computeIfAbsent(annotationType, Reflection::getDeclaredMethods);
+        synchronized (annotationType) { // cache is not thread safe generally
+            this.methods = Validate.notNull(cache, "cache").computeIfAbsent(annotationType, Reflection::getDeclaredMethods);
+        }
     }
 
     /**
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/LRUCache.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/LRUCache.java
index 48fcd7d..112ddf2 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/LRUCache.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/LRUCache.java
@@ -35,7 +35,7 @@ public class LRUCache<K, V> extends LinkedHashMap<K, V> {
     }
 
     @Override
-    protected boolean removeEldestEntry(Map.Entry<K, V> eldest) {
+    protected boolean removeEldestEntry(Map.Entry<K, V> eldest) { // todo: synchronize?
         return super.removeEldestEntry(eldest) || size() >= maximumCapacity;
     }
 }