CODEWITHSUNDEEP
javamultithreadingcollections
St.Antario
St.Antario

Reputation: 27375

Synchronized collection vs synchronized method?

I have a class container containing a collection which is going to be used by multiple threads:

public class Container{

    private Map<String, String> map;

    //ctor, other methods reading the map

    public void doSomeWithMap(String key, String value){
        //do some threads safe action
        map.put(key, value);
        //do something else, also thread safe
    }
}

What would be better, to declare the method synchronized:

public synchronized void doSomeWithMap(String key, String value)

or to use standard thread-safe decorator?

Collections.synchronizedMap(map);

Upvotes: 3

Views: 1387

Answers (5)

Karthik R
Karthik R

Reputation: 5786

You should probably have the block to be synchronized upon the requirement. Please note this.

When you use synchronized Collection like ConcurrentHashMap or Collection's method like synchronizedMap(), synchronizedList() etc, only the Map/List is synchronized. To explain little further,

Consider,

Map<String, Object> map = new HashMap<>();
Map<String, Object> synchMap = Collections.synchronizedMap(map);

This makes the map's get operation to be synchronous and not the objects inside it.

Object o = synchMap.get("1");// Object referenced by o is not synchronized. Only the map is.

If you want to protect the objects inside the Map, then you also have to put the code inside the synchronized block. This is good to remember as many people forget to safe guard the object in most cases.

Look at this for little info too Collection's synchronizedMap

Upvotes: 1

Raffaele
Raffaele

Reputation: 20875

If you look at the implementation of SynchronizedMap, you'll see that it's simply a map wrapping a non thread-safe map that uses a mutex before calling any method

public V get(Object key) {
  synchronized (mutex) {return m.get(key);}
}

public V put(K key, V value) {
  synchronized (mutex) {return m.put(key, value);}
}

public Set<Map.Entry<K,V>> entrySet() {
  synchronized (mutex) {
    if (entrySet==null)
      entrySet = new SynchronizedSet<>(m.entrySet(), mutex);
    return entrySet;
  }
}

If all you want is protecting get and put, this implementation does it for you.

However it's not suitable if you want a Map that can be iterated over and updated by two or more threads, in which case you should use a ConcurrentHashMap.

Upvotes: 2

Tim Bender
Tim Bender

Reputation: 20442

Generally speaking, synchronizing the map will protect most access to it without having to think about it further. However, the "synchronized map" is not safe for iteration which may be an issue depending on your use case. It is imperative that the user manually synchronize on the returned map when iterating over any of its collection views.

Consider using ConcurrentHashMap if that will meet your use case.

If there is other state to this object that needs to be protected from concurrency errors, then you will need to use synchronized or a Lock.

Upvotes: 2

Andreas
Andreas

Reputation: 159086

If your doSomeWithMap method will access the map more than once, you must synchronize the doSomeWithMap method. If the only access is the put() call shown, then it's better to use a ConcurrentHashMap.

Note that "more than once" is any call, and an iterator is by nature many "gets".

Upvotes: 2

codebox
codebox

Reputation: 20254

If the other things you do inside doSomeWithMap would cause problems if done concurrently by different threads (for example, they update class-level variables) then you should synchronise the whole method. If this is not the case then you should use a synchronised Map in order to minimise the length of time the synchronisation lock is left in place.

Upvotes: 1

Related Questions