CODEWITHSUNDEEP
javaconcurrency
ses
ses

Reputation: 13372

Using iterator over for-loop in concurrent java app

class Nodes has getNodes() method, which is not synchronized. But List<Node> nodes - is synchronized. Many threads could be connected to it, changing nodes in it.

Like this:

class Nodes {
 List<Node> nodes = Collections.synchronizedList(new ArrayList<Node>() );

 public List<Nodes> getNodes() { return nodes; }
 ...
}

Client code:

Nodes nodes;

synchronized(nodes) {

 for(Node node: nodes.getNodes()) {
  ...
 }

}

I do not have interrogation tests for that, but:

Should I use while(iterator.hasNext()) { var = iterator.next() } instead of for-loop ?

Because I know that when I try to delete nodes.remove(node) inside for-loop it fails with ConcurentModificationException.

EDIT: (related issue)

If iterator is good stuff to use, then having this code (client code):

Iterator<Node> iter = nodes.getNodes().iterator();
while (iter.hasNext()) {  // line 1
    Node node = iter.next();  // line 2
}

It is not safe anyway:

 1. thread1 goes to line 1, hoping that now iter would return him next() value. 
 2. but at that moment thread2 delete that value.
 3. thread1 has Exception and fails.

Does it mean that I should do locking on client side anyway. This is what I don't want to do.

One of the solutions I have:

while (iter.hasNext()) {
            
    try {
       Node node = iter.next();
       ...
                  
    } catch (NoSuchElementException ex) {continue;}  // handle exception - do more try
}

EDIT:

Answer for my case was: to use CopyOnWriteArrayList. I can even stay with for-loop with it.

But another option: Just return client a copy of the list to let them know whatever they want with it. Because it is kind of strange (inconsistent) providing 'snapshot iterator' AND real data in the list at the same time.

Upvotes: 2

Views: 1737

Answers (3)

ses
ses

Reputation: 13372

What is even better:

To use:

private List<Node> defensiveCopyNodeList() {
    List<Node> nodesListCopy = Lists.newLinkedList();
    synchronized (nodesList) {
        nodesListCopy = ImmutableList.copyOf(nodesList);  // Google [Guava lib][1]
    }
    return  nodesListCopy;
}

Then in getter:

public List<Node> getNodes() {
      return  defensiveCopyNodeList();
}

Then it allows us to use safely not only iterator but and data itself.

Upvotes: 0

rolfl
rolfl

Reputation: 17705

You should use an iterator like you have suggest, but instead of doing a nodes.delete() (which is really a nodes.remove(...) ) you should instead do iterator.remove()

You have updated your question. Here's an updated answer addressing the 'atomicity' of the iterator. If you want your iterator to have a 'snapshot' of the values at the time it (the iterator) was created, then you can use the Concurrent set of collections in java.util.concurrent: like CopyOnWriteArrayList

Upvotes: 2

Nick
Nick

Reputation: 2845

Iterator.remove is the only safe way to modify a collection during iteration

Source: The Collection Interface tutorial

Upvotes: 4

Related Questions