From f047eecc72789d30bd740469b1517242e9afcf9e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Finn=C3=A9?= <mattias@neotechnology.com>
Date: Wed, 20 Sep 2017 12:48:30 +0200
Subject: [PATCH] Fixes broken concurrency in SchemaCache

Previously there were only some data structures (maps/sets) that used implementations
which could handle concurrent access. Seemingly that was added ad-hoc.
Originally the state in this class was meant to not care about concurrency
and instead rely on external synchronization. However this was never fully
the case since there could be unguarded reads happening concurrently with updates
to these non-concurrency-aware data structures, which means that some existing
data could be observed as missing when other unrelated data items were removed
or added. This was indeed proven by ConcurrentCreateDropIndexIT in an upcoming
commit.
---
 .../kernel/impl/api/store/SchemaCache.java     | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SchemaCache.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SchemaCache.java
index d73b38670a62c..9ddb5414ac960 100644
--- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SchemaCache.java
+++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SchemaCache.java
@@ -19,7 +19,6 @@
  */
 package org.neo4j.kernel.impl.api.store;
 
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -29,8 +28,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 
-import org.neo4j.collection.primitive.Primitive;
-import org.neo4j.collection.primitive.PrimitiveIntObjectMap;
 import org.neo4j.helpers.collection.Iterators;
 import org.neo4j.kernel.api.schema.LabelSchemaDescriptor;
 import org.neo4j.kernel.api.schema.SchemaDescriptor;
@@ -43,6 +40,7 @@
 import org.neo4j.storageengine.api.schema.SchemaRule;
 
 import static java.util.Collections.emptyIterator;
+import static java.util.Collections.newSetFromMap;
 
 /**
  * A cache of {@link SchemaRule schema rules} as well as enforcement of schema consistency.
@@ -55,16 +53,16 @@
  */
 public class SchemaCache
 {
-    private final Map<Long, IndexRule> indexRuleById = new HashMap<>();
-    private final Map<Long, ConstraintRule> constraintRuleById = new HashMap<>();
-    private final Set<ConstraintDescriptor> constraints = new HashSet<>();
+    private final Map<Long,IndexRule> indexRuleById = new ConcurrentHashMap<>();
+    private final Map<Long,ConstraintRule> constraintRuleById = new ConcurrentHashMap<>();
+    private final Set<ConstraintDescriptor> constraints = newSetFromMap( new ConcurrentHashMap<>() );
 
-    private final Map<SchemaDescriptor,IndexDescriptor> indexDescriptors = new HashMap<>();
-    private final PrimitiveIntObjectMap<Set<IndexDescriptor>> indexDescriptorsByLabel = Primitive.intObjectMap();
+    private final Map<SchemaDescriptor,IndexDescriptor> indexDescriptors = new ConcurrentHashMap<>();
+    private final Map<Integer,Set<IndexDescriptor>> indexDescriptorsByLabel = new ConcurrentHashMap<>();
     private final ConstraintSemantics constraintSemantics;
 
     private final Map<Class<?>,Object> dependantState = new ConcurrentHashMap<>();
-    private final PrimitiveIntObjectMap<List<IndexDescriptor>> indexByProperty = Primitive.intObjectMap();
+    private final Map<Integer,List<IndexDescriptor>> indexByProperty = new ConcurrentHashMap<>();
 
     public SchemaCache( ConstraintSemantics constraintSemantics, Iterable<SchemaRule> initialRules )
     {
@@ -204,7 +202,7 @@ public void removeSchemaRule( long id )
         if ( constraintRuleById.containsKey( id ) )
         {
             ConstraintRule rule = constraintRuleById.remove( id );
-            constraints.remove( constraintSemantics.readConstraint( rule ) );
+            constraints.remove( rule.getConstraintDescriptor() );
         }
         else if ( indexRuleById.containsKey( id ) )
         {