From 727e6172e3f98ef31b48ba4780c28b8cc4e89585 Mon Sep 17 00:00:00 2001
From: Adrien Grand <jpountz@gmail.com>
Date: Mon, 5 May 2014 09:52:40 +0200
Subject: [PATCH] Restore read/write visibility is PlainShardsIterator.

Change #5561 introduced a potential bug in that iterations that are performed
on a thread are might not be visible to other threads due to the removal of the
`volatile` keyword.

Close #6039
---
 .../cluster/routing/PlainShardsIterator.java   | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java b/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java
index b3dc44c2fc2..78054b53450 100644
--- a/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java
+++ b/src/main/java/org/elasticsearch/cluster/routing/PlainShardsIterator.java
@@ -19,7 +19,6 @@
 package org.elasticsearch.cluster.routing;
 
 import java.util.List;
-import java.util.ListIterator;
 
 /**
  * A simple {@link ShardsIterator} that iterates a list or sub-list of
@@ -29,21 +28,24 @@ public class PlainShardsIterator implements ShardsIterator {
 
     private final List<ShardRouting> shards;
 
-    private ListIterator<ShardRouting> iterator;
+    // Calls to nextOrNull might be performed on different threads in the transport actions so we need the volatile
+    // keyword in order to ensure visibility. Note that it is fine to use `volatile` for a counter in that case given
+    // that although nextOrNull might be called from different threads, it can never happen concurrently.
+    private volatile int index;
 
     public PlainShardsIterator(List<ShardRouting> shards) {
         this.shards = shards;
-        this.iterator = shards.listIterator();
+        reset();
     }
 
     @Override
     public void reset() {
-        iterator = shards.listIterator();
+        index = 0;
     }
 
     @Override
     public int remaining() {
-        return shards.size() - iterator.nextIndex();
+        return shards.size() - index;
     }
 
     @Override
@@ -56,10 +58,10 @@ public class PlainShardsIterator implements ShardsIterator {
 
     @Override
     public ShardRouting nextOrNull() {
-        if (iterator.hasNext()) {
-            return iterator.next();
-        } else {
+        if (index == shards.size()) {
             return null;
+        } else {
+            return shards.get(index++);
         }
     }