Skip to content

Commit

Permalink
Restore read/write visibility is PlainShardsIterator.
Browse files Browse the repository at this point in the history
Change elastic#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 elastic#6039
  • Loading branch information
jpountz committed May 5, 2014
1 parent 403824d commit 7d15688
Showing 1 changed file with 10 additions and 8 deletions.
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -56,10 +58,10 @@ public ShardRouting firstOrNull() {

@Override
public ShardRouting nextOrNull() {
if (iterator.hasNext()) {
return iterator.next();
} else {
if (index == shards.size()) {
return null;
} else {
return shards.get(index++);
}
}

Expand Down

0 comments on commit 7d15688

Please sign in to comment.