Java Concurrency: thread-safe modification of values in maps

Question

I'm having a bit of trouble concerning concurrency and maps in Java. Basically I have multiple threads using (reading and modifying) their own maps, however each of these maps is a part of a larger map which is being read and modified by a further thread:

My main method creates all threads, the threads create their respective maps which are then put into the "main" map:

Map<String, MyObject> mainMap = new HashMap<String, Integer>();
FirstThread t1 = new FirstThread();
mainMap.putAll(t1.getMap());
t1.start();
SecondThread t2 = new SecondThread();
mainMap.putAll(t2.getMap());
t2.start();
ThirdThread t3 = new ThirdThread(mainMap);
t3.start();

The problem I'm facing now is that the third (main) thread sees arbitrary values in the map, depending on when one or both of the other threads update "their" items. I must however guarantee that the third thread can iterate over - and use the values of - the map without having to fear that a part of what is being read is "old":

FirstThread (analogue to SecondThread):

for (MyObject o : map.values()) {
    o.setNewValue(getNewValue());
}

ThirdThread:

for (MyObject o : map.values()) {
    doSomethingWith(o.getNewValue());
}

Any ideas? I've considered using a globally accessible (static final Object through a static class) lock which will be synchronized in each thread when the map must be modified. Or are there specific Map implementations that assess this particular problem which I could use?

Thanks in advance!

Edit: As suggested by @Pyranja, it would be possible to synchronize the getNewValue() method. However I forgot to mention that I am in fact trying to do something along the lines of transactions, where t1 and t2 modify multiple values before/after t3 works with said values. t3 is implemented in such a way that doSomethingWith() will not actually do anything with the value if it hasn't changed.

Answer 1

To synchronize at a higher level than the individual value objects, you need locks to handle the synchronization between the various threads. One way to do this, without changing your code too much, is a ReadWriteLock. Thread 1 and Thread 2 are writers, Thread 3 is a reader.

You can either do this with two locks, or one. I've sketched out below doing it with one lock, two writer threads, and one reader thread, without worrying about what happens with an exception during data update (ie, transaction rollback...).

All that said, this sounds like a classic producer-consumer scenario. You should consider using something like a BlockingQueue for communication between threads, as is outlined in this question.

There's other things you may want to consider changing as well, like using Runnable instead of extending Thread.

private static final class Value {

    public void update() {

    }

}

private static final class Key {

}

private final class MyReaderThread extends Thread {

    private final Map<Key, Value> allValues;

    public MyReaderThread(Map<Key, Value> allValues) {
        this.allValues = allValues;
    }

    @Override
    public void run() {
        while (!isInterrupted()) {
            readData();
        }
    }

    private void readData() {
        readLock.lock();
        try {
            for (Value value : allValues.values()) {
                // Do something
            }
        }
        finally {
            readLock.unlock();
        }

    }
}

private final class WriterThread extends Thread {

    private final Map<Key, Value> data = new HashMap<Key, Value>();

    @Override
    public void run() {
        while (!isInterrupted()) {
            writeData();
        }
    }

    private void writeData() {
        writeLock.lock();

        try {
            for (Value value : data.values()) {
                value.update();
            }
        }
        finally {
            writeLock.unlock();
        }
    }
}

private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();

private final ReadLock readLock;
private final WriteLock writeLock;

public Thing() {
    readLock = lock.readLock();
    writeLock = lock.writeLock();
}

public void doStuff() {
    WriterThread thread1 = new WriterThread();
    WriterThread thread2 = new WriterThread();

    Map<Key, Value> allValues = new HashMap<Key, Value>();
    allValues.putAll(thread1.data);
    allValues.putAll(thread2.data);
    MyReaderThread thread3 = new MyReaderThread(allValues);

    thread1.start();
    thread2.start();
    thread3.start();
}