How do I make this class threadsafe?

Question

Vector is thread-safe but in this implementation it doesn't seem to work. Making the methods synchronized doesn't help either, java.util.ConcurrentModificationException is thrown.

What could be done to fix it for this very implementation?

Should I go for java.util.concurrent.CopyOnWriteArrayList?

import java.util.Iterator; import java.util.Vector; public class VectorExample { private Vector<Object> v; public VectorExample() { v = new Vector<Object>(); } public void addToVector(Object o) { v.add(o); } public Vector<Object> getV() { return v; } public static void main(String[] args) { final VectorExample v = new VectorExample(); v.addToVector("obj1"); v.addToVector("obj2"); v.addToVector("obj3"); v.addToVector("obj4"); v.addToVector("obj5"); v.addToVector("obj6"); new Thread(new Runnable() { @Override public void run() { Iterator<Object> it = v.getV().iterator(); while(it.hasNext()) { System.out.println(it.next().toString()); } } }).start(); new Thread(new Runnable() { @Override public void run() { v.addToVector("Obj11"); v.addToVector("Obj12"); v.addToVector("Obj13"); v.addToVector("Obj14"); v.addToVector("Obj15"); v.addToVector("Obj16"); } }).start(); } }

Mike Samuel · Accepted Answer · 2013-04-22 18:30:53Z

The problem is not solvable by simply adding synchronization to existing methods, because it involves interleaving uses of an iterator and mutation.

Javadoc

it is not generally permissible for one thread to modify a Collection while another thread is iterating over it. In general, the results of the iteration are undefined under these circumstances. Some Iterator implementations (including those of all the general purpose collection implementations provided by the JRE) may choose to throw this exception if this behavior is detected. Iterators that do this are known as fail-fast iterators, as they fail quickly and cleanly, rather that risking arbitrary, non-deterministic behavior at an undetermined time in the future.

Putting

 synchronized (v) { ... }

around the bodies of the first run() method and making addToVector synchronized will solve the immediate problem by ensuring that the entire life of the iterator is mutually exclusive with mutations to the vector.

To solve the larger problem, clients needing to know to synchronize over the whole lifetime of an iterator, you should probably expose a synchronized fold method instead of exposing the iterator. That way, each reader doesn't need to synchronize their entire read.

interface Folder<IN, OUT> { OUT foldOne(OUT x, IN element); } <OUT> synchronized OUT foldLeft(Folder<? super Object, OUT> folder, OUT x) { for (Object element : v) { x = folder.foldOne(x, element); } return x; }

Alternatively, if iteration is an infrequent operation, just return an iterator into a copy of the vector.

Iterators are fail fast and throw ConcurrentModificationException, I removed synchronized from the methods and used a simple for loop in the first run method public void run() { int x=v.getV().size(); for(int i=0;i<x;i++) { System.out.println(v.getV().elementAt(i)); }} The results this time were consistent for every run. What exactly happened? Does it make the class thread-safe?
@n1234, index loops are not fail fast, so the reader thread is seeing some but not all of the elements of the vector, and doing so silently. If you added items in the main thread, and removed in the child thread instead of adding, then the for loop would often fail with an IndexOutOfBoundsException because x would be greater than the size of the vector late in the loop.

Steinar · Accepted Answer · 2013-04-22 18:23:48Z

Looks like you're getting the exception because you're iterating and modifying the collection simultaneously. You could try cloning the collection and iterate the clone instead.

xtraclass · Accepted Answer · 2013-04-22 18:25:44Z

One possibility: put on synchronized block in each run run method:

 synchronized ( v ) { Iterator<Object> it = v.getV().iterator(); while(it.hasNext()) { System.out.println(it.next().toString()); } } synchronized ( v ) { v.addToVector("Obj11"); v.addToVector("Obj12"); v.addToVector("Obj13"); v.addToVector("Obj14"); v.addToVector("Obj15"); v.addToVector("Obj16"); }

Another hint:

The code of the first run method could be in the VectorExample, like dump() or whatever. This would make the synchronization easier.

Mel Nicholson · Accepted Answer · 2013-04-22 18:22:49Z

The problem is that that you need to protect the whole iteration, not just the get().

The iterator from the Vector class becomes invalid when the Vector is written to. It will throw a ConcurrentModificationException if used afterwards.

There are two ways to fix this. You can synchornize the code block containing the loop and your add calls on the same lock object, or you can use a different collection which allows iteration to complete on the old data after new data is input.

Nathanial · Accepted Answer · 2013-04-22 18:26:09Z

You can not modify a collection while iterating over it. You can either make a copy of the collection and iterate over the copy or synchronize both blocks on the collection.

synchronized(v) { .. }

Collectives™ on Stack Overflow

How do I make this class threadsafe?

5 Answers 5

2 Comments

Comments

Comments

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

Comments

Comments

Comments

Comments

Related