Redundant synchronization?

[David Duran]

I'm still trying to get a grasp of this whole synchronization stuff. If I have a function which uses a synchronized object and is called from multiple threads, is it necessary make the method synchronized or is that redundant?
ie. a HashTable is already synchronized:
public synchronized void modify()
{
m_hashTable.put(new Object(), "key");
}
-------------------------------
On the flip-side, if I'm using an un-synchronized object, are all 3 of these the same?:
1. Synchronized method
public synchronized void modify()
{
m_hashMap.put(new Object(), "key");
}
2. Synchronized code block
public void modify()
{
synchronized(m_hashMap)
{
m_hashMap.put(new Object(), "key");
}
}
3. Synchronized object
m_hashMap = Collections.synchronizedMap(new HashMap());
...
public void modify()
{
m_hashMap.put(new Object(), "key");
}
[ March 15, 2002: Message edited by: David Duran ]

[Karthik Guru]

IMHO,

  
ie. a HashTable is already synchronized:
public synchronized void modify()
{    
m_hashTable.put(new Object(), "key");
}

the synchronized keyword is redundant in this case since a thread has to obtain a lock on
m_hashtable to invoke a put. So modify() need not be synchronized.

  
1. [I]Synchronized method[/I]
public synchronized void modify()
{
m_hashMap.put(new Object(), "key");
}
2. [I]Synchronized code block[/I]
public void modify()
{
synchronized(m_hashMap)
{
m_hashMap.put(new Object(), "key");
}
}
3. [I]Synchronized object[/I]
m_hashMap = Collections.synchronizedMap(new HashMap());
...
public void modify()
{
m_hashMap.put(new Object(), "key");
}

I think all the 3 code snippets try to achieve the same. ie synchronize access to a hashtable instance.

[Peter den Haan]

Agreed, with one important note: method 1), the synchronized method, might not achieve what you want since it does not synchronize on the Map. "Might"? Well, if all threads accessing the Map use synchronized methods on the same object, you're OK. On the other hand, if they use other means of access then synchronzing the method won't achieve anything meaningful.
You may find this thread interesting reading as well.
- Peter

[Karthik Guru]

Originally posted by Peter den Haan:
You may find this thread interesting reading as well.
- Peter

yes a nice one.

[David Duran]

I got this from the javadoc:

Map m = Collections.synchronizedMap(new HashMap());
...
Set s = m.keySet();  // Needn't be in synchronized block
...
synchronized(m) {  // Synchronizing on m, not s!
   Iterator i = s.iterator(); // Must be in synchronized block
   while (i.hasNext())
      foo(i.next());
}

Is it necessary that the above synchronized segment be based on a synchronized Map object? That seems redundant. In other words, would it have been okay to replace the first line with:
Map m = new HashMap(); // not synchronized

[John Dale]

When you look at synchronization, pay attention to what objects are the target of the synchronization, and think of a synchronized method as though it its block of code were "synchronized(this)". Here is a crude example that may help illustrate the point.
For example

  
class Demo {
final HashTable ht = new Hashtable();
void define(Object o){
 synchronized(this) {
  ht.put(o, "");
 }
};
void another(Object o) {
 synchronized(this) {
  String s = (String)ht.get(o);
  if (s == null) //1
    define(o); // 2
  else {
    ht.put(o, s+"."); // 3
  }
 }
};
}

Suppose we have one instance demo of Demo, and we get the following sequence of calls
Thread A calls demo.define("X"), which runs to completion.
Thread A calls demo.another("X"), and gets pre-empted between the lines marked 1 and 3.
Now thread B executes and calls demo.another("X").
As written, the map would contain X="..".
If we did not include the synchronzed(this) in both define() and another(), then the another() called by thread B could run in the between the lines marked 1 and 3 of thread A's call to "another", resulting in the final value X=".".
In this case, it is important that the body of both methods be explicitly synchronized on the same object, whether it be the Hashtable object or the Demo object; I chose the demo object here. If and only if no other methods of Demo or any other class refer to this hashtable without being synchronized on the same object, then it does not matter whether we used a synchronized collection (e.g., Hashtable) or an unsynchronized collection (e.g., HashArray).
For the purpose of this discussion, I chose to write out that the bodies of the methods are synchronized on this. Of couse, this is equivalent to using the synchronized modifier in the method headers instead.

[John Dale]

In response to your last question (paraphrased): What happens if I use the synchronized block to iterate over an unsynchronized map instead of a synchronized map?
Consider what happens if another thread calls map.put(key, value) while your thread is iterating. If the map isn't synchronized, you may have an unintended concurrent access.

[Mr. C Lamont Gilbert]

Its all a matter of what you want to synchronize. Synchronized methods are use to prevent "other methods" from executing while that one is. So if you had 2 methods
public synchronized void getIt() {}
and
public synchronized void putIt() {}
Those 2 would not run at the same time. So if you are noticying that their are LOTS of ways to synchronize, you are correct. Their is no best way except the way that works. Sometimes circumstances allow you MANY ways to synchronize, and at other times you will find that circumstances force you into a particular format of synchronization.
I personally use the HashMap and syhcnronize my method because sometimes I access it without any synchronization when I know its safe, and that makes it faster. (err, is that HashMap or HashTable that is not synchronized??? )