Handle thread safety in part creation

neo4j · Mar 27, 2018 · 64c83ea · 64c83ea
1 parent 3659658
commit 64c83ea
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 50 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialIndexCache.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialIndexCache.java
@@ -73,7 +73,7 @@ T uncheckedSelect( CoordinateReferenceSystem crs )
      * @return selected part
      * @throws E exception potentially thrown during creation
      */
-    T select( CoordinateReferenceSystem crs ) throws E
+    synchronized T select( CoordinateReferenceSystem crs ) throws E
     {
         T part = spatials.get( crs );
         if ( part == null )

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCache.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/TemporalIndexCache.java
@@ -19,13 +19,15 @@
  */
 package org.neo4j.kernel.impl.index.schema;
 
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
-import java.util.List;
+import java.util.Objects;
 import java.util.function.Function;
 
 import org.neo4j.values.storable.ValueGroup;
 
+import static org.neo4j.kernel.impl.index.schema.TemporalIndexCache.Offset.*;
+
 /**
  * Cache for lazily creating parts of the temporal index. Each part is created using the factory
  * the first time it is selected in a select() query, or the first time it's explicitly
@@ -40,19 +42,30 @@ class TemporalIndexCache<T, E extends Exception> implements Iterable<T>
 {
     private final Factory<T, E> factory;
 
-    private volatile T date;
+    enum Offset
-    private volatile T localDateTime;
+    {
-    private volatile T zonedDateTime;
+        date,
-    private volatile T localTime;
+        localDateTime,
-    private volatile T zonedTime;
+        zonedDateTime,
-    private volatile T duration;
+        localTime,
+        zonedTime,
+        duration
+    }
 
-    private List<T> parts;
+    private final Object dateLock = new Object();
+    private final Object localDateTimeLock = new Object();
+    private final Object zonedDateTimeLock = new Object();
+    private final Object localTimeLock = new Object();
+    private final Object zonedTimeLock = new Object();
+    private final Object durationLock = new Object();
+
+    private T[] parts;
 
     TemporalIndexCache( Factory<T, E> factory )
     {
         this.factory = factory;
-        this.parts = new ArrayList<>();
+        //noinspection unchecked
+        this.parts = (T[]) new Object[Offset.values().length];
     }
 
     /**
@@ -124,22 +137,22 @@ <RESULT> RESULT selectOrElse( ValueGroup valueGroup, Function<T, RESULT> functio
         switch ( valueGroup )
         {
         case DATE:
-            return date != null ? function.apply( date ) : orElse;
+            return parts[date.ordinal()] != null ? function.apply( parts[date.ordinal()] ) : orElse;
 
         case LOCAL_DATE_TIME:
-            return localDateTime != null ? function.apply( localDateTime ) : orElse;
+            return parts[localDateTime.ordinal()] != null ? function.apply( parts[localDateTime.ordinal()] ) : orElse;
 
         case ZONED_DATE_TIME:
-            return zonedDateTime != null ? function.apply( zonedDateTime ) : orElse;
+            return parts[zonedDateTime.ordinal()] != null ? function.apply( parts[zonedDateTime.ordinal()] ) : orElse;
 
         case LOCAL_TIME:
-            return localTime != null ? function.apply( localTime ) : orElse;
+            return parts[localTime.ordinal()] != null ? function.apply( parts[localTime.ordinal()] ) : orElse;
 
         case ZONED_TIME:
-            return zonedTime != null ? function.apply( zonedTime ) : orElse;
+            return parts[zonedTime.ordinal()] != null ? function.apply( parts[zonedTime.ordinal()] ) : orElse;
 
         case DURATION:
-            return duration != null ? function.apply( duration ) : orElse;
+            return parts[duration.ordinal()] != null ? function.apply( parts[duration.ordinal()] ) : orElse;
 
         default:
             throw new IllegalStateException( "Unsupported value group " + valueGroup );
@@ -148,70 +161,74 @@ <RESULT> RESULT selectOrElse( ValueGroup valueGroup, Function<T, RESULT> functio
 
     T date() throws E
     {
-        if ( date == null )
+        synchronized ( dateLock )
         {
-            date = factory.newDate();
+            if ( parts[date.ordinal()] == null )
-            addPartToList( date );
+            {
+                parts[date.ordinal()] = factory.newDate();
+            }
         }
-        return date;
+        return parts[date.ordinal()];
     }
 
     T localDateTime() throws E
     {
-        if ( localDateTime == null )
+        synchronized ( localDateTimeLock )
         {
-            localDateTime = factory.newLocalDateTime();
+            if ( parts[localDateTime.ordinal()] == null )
-            addPartToList( localDateTime );
+            {
+                parts[localDateTime.ordinal()] = factory.newLocalDateTime();
+            }
         }
-        return localDateTime;
+        return parts[localDateTime.ordinal()];
     }
 
     T zonedDateTime() throws E
     {
-        if ( zonedDateTime == null )
+        synchronized ( zonedDateTimeLock )
         {
-            zonedDateTime = factory.newZonedDateTime();
+            if ( parts[zonedDateTime.ordinal()] == null )
-            addPartToList( zonedDateTime );
+            {
+                parts[zonedDateTime.ordinal()] = factory.newZonedDateTime();
+            }
         }
-        return zonedDateTime;
+        return parts[zonedDateTime.ordinal()];
     }
 
     T localTime() throws E
     {
-        if ( localTime == null )
+        synchronized ( localTimeLock )
         {
-            localTime = factory.newLocalTime();
+            if ( parts[localTime.ordinal()] == null )
-            addPartToList( localTime );
+            {
+                parts[localTime.ordinal()] = factory.newLocalTime();
+            }
         }
-        return localTime;
+        return parts[localTime.ordinal()];
     }
 
     T zonedTime() throws E
     {
-        if ( zonedTime == null )
+        synchronized ( zonedTimeLock )
         {
-            zonedTime = factory.newZonedTime();
+            if ( parts[zonedTime.ordinal()] == null )
-            addPartToList( zonedTime );
+            {
+                parts[zonedTime.ordinal()] = factory.newZonedTime();
+            }
         }
-        return zonedTime;
+        return parts[zonedTime.ordinal()];
     }
 
     T duration() throws E
     {
-        if ( duration == null )
+        synchronized ( durationLock )
-        {
-            duration = factory.newDuration();
-            addPartToList( duration );
-        }
-        return duration;
-    }
-
-    private void addPartToList( T t )
-    {
-        if ( t != null )
         {
-            parts.add( t );
+            if ( parts[duration.ordinal()] == null )
+            {
+                parts[duration.ordinal()] = factory.newDuration();
+            }
         }
+        return parts[duration.ordinal()];
     }
 
     void loadAll()
@@ -234,7 +251,7 @@ void loadAll()
     @Override
     public Iterator<T> iterator()
     {
-        return parts.iterator();
+        return Arrays.stream( parts ).filter( Objects::nonNull ).iterator();
     }
 
     /**