Returning a copied collection to avoid corrupted iterator by multiple…

… threads

Signed-off-by: ntisseyre <[email protected]>

janusgraph-core/src/main/java/org/janusgraph/graphdb/transaction/StandardJanusGraphTx.java

@@ -1568,7 +1568,7 @@ public synchronized void commit() {
         }
         try {
             if (hasModifications()) {
-                graph.commit(addedRelations.getAll(), deletedRelations.values(), this);
+                graph.commit(addedRelations.getAllUnsafe(), deletedRelations.values(), this);
             } else {
                 txHandle.commit();
             }

janusgraph-core/src/main/java/org/janusgraph/graphdb/transaction/addedrelations/AddedRelationsContainer.java

@@ -39,7 +39,7 @@ public interface AddedRelationsContainer {
      * of the transaction after there are no additional changes. Otherwise the behavior is non deterministic.
      * @return
      */
-    Collection<InternalRelation> getAll();
+    Collection<InternalRelation> getAllUnsafe();
 
     /**
      * Clears the container which releases allocated memory.
@@ -69,7 +69,7 @@ public boolean isEmpty() {
         }
 
         @Override
-        public Collection<InternalRelation> getAll() {
+        public Collection<InternalRelation> getAllUnsafe() {
             return Collections.emptyList();
         }
 

janusgraph-core/src/main/java/org/janusgraph/graphdb/transaction/addedrelations/ConcurrentAddedRelations.java

@@ -15,8 +15,10 @@
 package org.janusgraph.graphdb.transaction.addedrelations;
 
 import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
 import org.janusgraph.graphdb.internal.InternalRelation;
 
+import java.util.ArrayList;
 import java.util.Collection;
 
 /**
@@ -37,11 +39,17 @@ public synchronized boolean remove(final InternalRelation relation) {
 
     @Override
     public synchronized Iterable<InternalRelation> getView(final Predicate<InternalRelation> filter) {
-        return super.getView(filter);
+        return copyView(super.getView(filter));
     }
 
     @Override
-    public synchronized Collection<InternalRelation> getAll() {
-        return super.getAll();
+    public synchronized Collection<InternalRelation> getAllUnsafe() {
+        return super.getAllUnsafe();
+    }
+
+    private Iterable<InternalRelation> copyView(Iterable<InternalRelation> currentView) {
+        ArrayList<InternalRelation> view = new ArrayList<>();
+        Iterables.addAll(view, currentView);
+        return view;
     }
 }

janusgraph-core/src/main/java/org/janusgraph/graphdb/transaction/addedrelations/EmptyAddedRelations.java

@@ -52,7 +52,7 @@ public boolean isEmpty() {
     }
 
     @Override
-    public Collection<InternalRelation> getAll() {
+    public Collection<InternalRelation> getAllUnsafe() {
         return Collections.emptyList();
     }
 

janusgraph-core/src/main/java/org/janusgraph/graphdb/transaction/addedrelations/SimpleAddedRelations.java

@@ -62,7 +62,7 @@ public boolean isEmpty() {
     }
 
     @Override
-    public Collection<InternalRelation> getAll() {
+    public Collection<InternalRelation> getAllUnsafe() {
         return Collections.unmodifiableCollection(new AbstractCollection<InternalRelation>() {
             @Override
             @Nonnull

janusgraph-test/src/test/java/org/janusgraph/graphdb/transaction/addedrelations/EmptyAddedRelationsTest.java

@@ -41,8 +41,8 @@ public void isEmpty() {
     }
 
     @Test
-    public void getAllReturnsEmpty() {
-        assertTrue(EmptyAddedRelations.getInstance().getAll().isEmpty());
+    public void getAllUnsafeReturnsEmpty() {
+        assertTrue(EmptyAddedRelations.getInstance().getAllUnsafe().isEmpty());
     }
 
     @Test