Question

In my Java class I include a Hashmap variable (class property) and run some Threads which write-only into that HashMap using put(): each time the write happens it stores a unique key (which is done by design).

Is the synchronized keyword on a class method write-only sufficient for thead-safe conditions? My HashMap is simple and not a ConcurrentHashMap?

Was it helpful?

Solution

No, it is not sufficient to only synchronize the writes. Synchronization must be applied to both reads and writes to memory.

Some other thread, somewhere, sometime, will need to read the map (otherwise, why have a map?), and that thread needs to be synchronized to correctly view the memory represented by the map. They also need to be synchronized to avoid tripping over transient inconsistencies in the map state as it's being updated.

To provide a hypothetical example, suppose Thread 1 writes the hashmap, the effects of which are stored in CPU 1's level 1 cache only. Then Thread 2, becomes eligible to run a few seconds later and is resumed on CPU 2; it reads the hashmap, which comes from CPU 2's level 1 cache - it does not see the writes that Thread 1 made, because there was no memory barrier operation between the write and the read in both the writing and the reading thread. Even if Thread 1 synchronizes the writes, then although the effect of the writes will be flushed to main memory, Thread 2 will still not see them because the read came from level 1 cache. So synchronizing writes only prevents collisions on writes.

Besides the CPU caching the JMM allows threads to cache data privately themselves which only has to be flushed to main memory at a memory barrier (synchronize, volatile with some special limitations, or completion of construction of an immutable object in JMM 5+).

To fully understand this complex subject of threading you must research and study the Java Memory Model, and it's implications for sharing data between threads. You must understand the concepts of "happens-before" relationships and memory visibility to understand the complexities of sharing data in today's world of multicore CPUs with various levels of CPU caching.

If you don't want to invest the time to understand the JMM, the simple rule is that two threads must somewhere/somehow synchronize on the same object between the writes and the reads for one thread to see the effects of the operations of the other. Period. Note that this doesn't mean that all writes and reads on an object must be synchronized, per se; it is legitimate to create and configure an object in one thread and then "publish" it to other threads, as long as the publishing thread and fetching thread(s) synchronize on the same object for the hand over.

OTHER TIPS

You can just add the synchronized modifier to your method signature and it should be fine. I made a quick example to show you it in action. You can modify the loop to setup as many threads as you want.

It'll try to add the same key n times, and if you have a concurrency problem, the map should have duplicate keys in it.

class MyMap{

    private Map<String, Object> map;

    public MyMap(){
        map = new HashMap<String, Object>();
    }

    public synchronized void put(String key, Object value){
        map.put(key, value);
    }

    public Map<String, Object> getMap(){
        return map;
    }

}

class MyRunnable implements Runnable{

    private MyMap clazz;

    public MyRunnable(MyMap clazz){
        this.clazz = clazz;
    }

    @Override
    public void run(){
        clazz.put("1", "1");
    }

}

public class Test{

    public static void main(String[] args) throws Exception{
        MyMap c = new MyMap();

        for(int i = 0 ; i < 1000 ; i ++){
            new Thread(new MyRunnable(c)).start();
        }

        for(Map.Entry<String, Object> entry : c.getMap().entrySet()){
            System.out.println(entry);
        }
    }
}

The synchronized write method is sufficient for thread safety as long as:

  • No other method of your class allows to modify the underlying hash table;
  • The underlying hash table is not exposed in any way so it can't be modified by its own methods (easy: construct a private instance);
  • All methods that read the hash table are also synchronized if used at the same time as the write method. Imagine what may happen if a get() is called when the hash map is halfway being modified.

The last point sucks if you have to read from your hash map at the same time as you write into it; use ConcurrentHashMap in this case.

If you only have a bunch of concurrent writes into the hash map, and then read it in only one thread, your solution should be fine.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top