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

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()) {
+        if (index == shards.size()) {
-            return iterator.next();
-        } else {
             return null;
+        } else {
+            return shards.get(index++);
         }
     }