CODEWITHSUNDEEP
javathread-safety
Nishant
Nishant

Reputation: 1142

How do I make this class threadsafe?

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();    
    }
}

Upvotes: 2

Views: 125

Answers (5)

Mike Samuel
Mike Samuel

Reputation: 120576

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.

Upvotes: 2

Nathanial
Nathanial

Reputation: 2257

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) {
  ..
}

Upvotes: 0

xtraclass
xtraclass

Reputation: 445

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.

Upvotes: 2

Steinar
Steinar

Reputation: 5950

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.

Upvotes: 2

Mel Nicholson
Mel Nicholson

Reputation: 3225

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.

Upvotes: 0

Related Questions