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

Answer 2

ConcurrentHashMap from java.util.concurrent - a thread-safe implementation of Map, which provides a much higher degree of concurrency than synchronizedMap. Just a lot of reads can almost always be performed in parallel, simultaneous reads and writes can usually be done in parallel, and multiple simultaneous recordings can often be done in parallel. (The class ConcurrentReaderHashMap offers a similar parallelism for multiple read operations, but allows only one active write operation.) ConcurrentHashMapis designed to optimize the retrieval operations.

Answer 3

Your example code may be misleading. In your first example you create a HashMap<String,Integer> but the second part iterates the map values which in this case are MyObject. The key to synchronization is to understand where and which mutable state is shared.

An Integer is immutable. It can be shared freely (but the reference to an Integer is mutable - it must be safely publicated and/or synchronized). But your code example suggests that the maps are populated with mutable MyObject instances.

Given that the map entries (key -> MyObject references) are not changed by any thread and all maps are created and safely publicated before any thread starts it would be in my opinion sufficient to synchronize the modification of MyObject. E.g.:

public class MyObject {
   private Object value;

   synchronized Object getNewValue() {
      return value;
   }

   synchronized void setNewValue(final Object newValue) {
      this.value = newValue;
   }
}

If my assumptions are not correct, clarify your question / code example and also consider @jacobm's comment and @Alex answer.