views:

281

answers:

5

If I'm using ConcurrentHashMap (where the put is thread safe) , and I supply a public function myPut that uses the ConcurrentHashMap put - do I need to synchronize my function?

meaning : should this be synchronized?

ConcurrentHashMap map;  
public void myPut(int something) {  
   this.map.put(something);  
}
A: 

No.
You can cal myPut multiple times at once.
The put will be called one at a time, because the HashTable's access needs to be one at a time. (see also concurrent writing in a shared memory)

Burkhard
+1  A: 

Hello,

If you use a ConcurrentHashMap you shouldn't synchronize 'put' but it doesn't mean your 'puts' will be called one a time. This depends on the concurrency level of you Map...

So you have to know what you want.

BTW have a look at the ConcurrentHashTable Javadoc, everything is well explained...

-Patrick

pgras
A: 

It depends.

When you are writing the classes for objects that will be used from multiple threads, the whole game changes. You need to understand what results you are trying to achieve if you hope to provide any thread safety guarantees.

In this case, if this is the only method in your class, there is no point in synchronizing access to this method. But it won't be the only method - for it would make the class pretty pointless.

You only need to synchronize this method if you need to do so to make the class thread safe for other reasons - at which point, you might want to question whether the overhead of a ConcurrentHashMap is worthwhile.

Bill Michell
+2  A: 

Because the map reference is not declared final it may be changed. Therefore, there is a potential threading bug here.

If map is supposed to be a mutable reference then you will need to do some more work. Otherwise use final. Indeed, use final whenever you can, even if it is "easier" not to. "final is the new [old] private." You probably want to make map private and generic too.

Tom Hawtin - tackline
+1  A: 

Concurrency utilities such as ConcurrentHashMap are designed so that you don't need to synchronize: they'll handle thread-safe access internally.

What Tom says is true, that you need to think about the potential of the map reference to change. If the reference actually doesn't change, then in practice you'll get away with it here: the internal synchronization of ConcurrentHashMap -- and indeed the java.util.concurrentlibrary in general -- guarantees that objects put into the map are safely published to other threads. But I would agree that even so, it is good practice to decide whether the reference can change or not, and then explicitly state this in the code ('final' if it can't; something like 'volatile' or an AtomicReference if it can).

Neil Coffey