Oded
Oded

Reputation: 664

java synchronizedMap operations on map objects should be synchronized

my code uses the following:

public class Obj{
   public String someOperation(){...}
};

public class ClassA{
   private Map<Integer, Object> m_MsgHash;

   public boolean init()
   {
      m_MsgHash = Collections.synchronizedMap(new LinkedHashMap<Integer, Object>(1001, 1.0F, true));
   }
   private Object fetchFromHash(int Id)
   {
     return m_MsgHash.get(Id);
   }

   public void HandleMsg(int Id)
   {
     Object obj = fetchFromHash(Id);
    // do some operation on obj needs to be synchronized ?
    //synchronized (m_MsgHash) {
    obj.someOperation();
    //}
   }
}

I understand from Java Doc that once iterating the my m_MsgHash i must use the synchronized keyword. but my question is, do i need to use the synchronized when using a fetched Object from my map ?

Upvotes: 0

Views: 189

Answers (3)

shazin
shazin

Reputation: 21883

Not required. Whenever you call Collections.synchronizedMap It creates a class that implements Map interface and has all the methods synchronized. This is called Java Monitor Pattern, where the underlying LinkedHashMap is protected by a Java Monitor to enable thread safety. You need to synchronize while looping because the Map may change while looping through.

But actions like put, get and remove are protected by the class Monitor thus not required to be inside of synchronized methods, unless they are part of a composite action such as Check-Then-Act.

Upvotes: 0

assylias
assylias

Reputation: 328618

No you don't: m_MsgHash.get(Id); is synchronized so it is a thread safe operation. And once you have a reference to obj you can do whatever you want with it without needing to synchronize as it lives independently from the map (as long as you don't share it across threads, but here it is a local variable).

Note however that your map is not safely published as it is. If a thread calls init and another calls HandleMsg, it is possible that the second thread sees a null value for the map.

A simple way to safely publish the map would be to make it final and instantiate it within the constructor of ClassA.

Upvotes: 2

Patricia Shanahan
Patricia Shanahan

Reputation: 26185

You do not need "synchronized" for simple operations on a synchronizedMap result, such as get. If the object referenced by obj is itself accessed from multiple threads, and modified by at least one of them, you need to make all accesses to it synchronized on the same object, or otherwise ensure multi-thread correctness.

Upvotes: 2

Related Questions