fix race condition in TinkerIndex

My colleage @fabsx00 discovered a race condition in tinkergraph's index 
creation. He fixed it by simply replacing parallelStream with stream. Quoting 
his analysis:
So, reading the code, you see that this.put is called in parallel, but that 
method seems to contain a race as get is called on the index, checked for null, 
and a subsequent write is performed. It still seems like using stream here 
fixes the problem we've been seeing, and the performance hit is not significant.

Ticket: https://issues.apache.org/jira/browse/TINKERPOP-1830


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/7be8a0f2
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/7be8a0f2
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/7be8a0f2

Branch: refs/heads/TINKERPOP-1489
Commit: 7be8a0f25672e20fb0fef58075c2a201501f0578
Parents: ac99e3c
Author: Michael Pollmeier <mich...@michaelpollmeier.com>
Authored: Sun Nov 12 12:32:37 2017 +1300
Committer: Michael Pollmeier <mich...@michaelpollmeier.com>
Committed: Tue Nov 14 14:22:40 2017 +1300

----------------------------------------------------------------------
 CHANGELOG.asciidoc                                       |  1 +
 .../gremlin/tinkergraph/structure/TinkerIndex.java       | 11 +++++------
 2 files changed, 6 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7be8a0f2/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index ffd9de5..cd5dc38 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -23,6 +23,7 @@ 
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 [[release-3-2-7]]
 === TinkerPop 3.2.7 (Release Date: NOT OFFICIALLY RELEASED YET)
 
+* Fixed a race condition in `TinkerIndex`.
 * Fixed an `ArrayOutOfBoundsException` in `hasId()` for the rare situation 
when the provided collection is empty.
 * Bump to Netty 4.0.52
 * `TraversalVertexProgram` `profile()` now accounts for worker iteration in 
`GraphComputer` OLAP.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7be8a0f2/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java
----------------------------------------------------------------------
diff --git 
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java
 
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java
index 69afb39..f5872cf 100644
--- 
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java
+++ 
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java
@@ -48,17 +48,16 @@ final class TinkerIndex<T extends Element> {
 
     protected void put(final String key, final Object value, final T element) {
         Map<Object, Set<T>> keyMap = this.index.get(key);
-        if (keyMap == null) {
-            keyMap = new ConcurrentHashMap<>();
-            this.index.put(key, keyMap);
+        if (null == keyMap) {
+            this.index.putIfAbsent(key, new ConcurrentHashMap<Object, 
Set<T>>());
+            keyMap = this.index.get(key);
         }
         Set<T> objects = keyMap.get(value);
         if (null == objects) {
-            objects = new HashSet<>();
-            keyMap.put(value, objects);
+            keyMap.putIfAbsent(value, ConcurrentHashMap.newKeySet());
+            objects = keyMap.get(value);
         }
         objects.add(element);
-
     }
 
     public List<T> get(final String key, final Object value) {

Reply via email to