nested locks

[VisionSet]

I'm aware that nesting synchronisation blocks is generally a bad idea.
Is this just due to the ease of deadlock?

Does this advice apply to implicit inner locks eg

Set mySyncedSet = Collections.synchronizedSet(new HashSet());
....
synchronized(this) {

mySyncedSet.add(anObj);

}

I guess in this case it would be fine because the Set is not going to hold
any locks for long.
I suppose the only problem would be if the inner lock does a loop on wait()
or blocks on I/O.
But the Javadoc(umentation) is not enforced in this area, it is not always
apparant if a method is synced or not, let alone the details of the objects
blocking.
Any guidelines available in this case?

 

[Rogan Dawes]

I'm aware that nesting synchronisation blocks is generally a bad idea.
Is this just due to the ease of deadlock?

Does this advice apply to implicit inner locks eg

Set mySyncedSet = Collections.synchronizedSet(new HashSet());
...
synchronized(this) {

mySyncedSet.add(anObj);

}

I guess in this case it would be fine because the Set is not going to hold
any locks for long.
I suppose the only problem would be if the inner lock does a loop on wait()
or blocks on I/O.
But the Javadoc(umentation) is not enforced in this area, it is not always
apparant if a method is synced or not, let alone the details of the objects
blocking.
Any guidelines available in this case?

My understanding is that the risks of nesting locks is that you could
deadlock, as you suggest. The other possibility is the added overhead
involved in actually synchronizing. If you can synchronize once, rather
than multiple times, that might be a better approach.

What I mean is, perhaps you don't need to make your set a
synchronizedSet, if accesses to the set are already done in synchronized
blocks . . .

Rogan

 

[VisionSet]

My understanding is that the risks of nesting locks is that you could
deadlock, as you suggest. The other possibility is the added overhead
involved in actually synchronizing. If you can synchronize once, rather
than multiple times, that might be a better approach.

What I mean is, perhaps you don't need to make your set a
synchronizedSet, if accesses to the set are already done in synchronized
blocks . . .

Yes, I appreciate what you are saying, my point is that when calling a
library method you may not know or be able to determine if there is any
internal synchronization on that method or what it invloves. You may
therefore inadvertently nest locks.

 

[Rogan Dawes]

Yes, I appreciate what you are saying, my point is that when calling a
library method you may not know or be able to determine if there is any
internal synchronization on that method or what it invloves. You may
therefore inadvertently nest locks.

In that case I'd say that if they do not tell you, it is because it
doesn't really matter . . . ;-)

Regards,

Rogan

 

[Michael Borgwardt]

I'm aware that nesting synchronisation blocks is generally a bad idea.
Is this just due to the ease of deadlock?
Yes.

Does this advice apply to implicit inner locks eg

Set mySyncedSet = Collections.synchronizedSet(new HashSet());
...
synchronized(this) {

mySyncedSet.add(anObj);

}

I guess in this case it would be fine because the Set is not going to hold
any locks for long.

No, it's fine because there almost certainly can be no deadlocks. Nested locks
result in deadlocks only if they are acquired in varying orders.

In your example, a deadlock could only occur if one of the synchronized Set methods
attempted to get a lock on "this", because then it coul happen that one thread has
the lock on "this" and waits for the lock on the set, while another thread has the
lock on the set and waits for the lock on "this".

The only way you could theoretically get a deadlock were if you added the "this" object
to the HashSet *and* if the class's equals() or hashCode() methods (which are called from
within HashSet) were synchronized *and* if at some point you called a method on the set
without first synchronizing on "this".

 

[VisionSet]

The only way you could theoretically get a deadlock were if you added the "this" object
to the HashSet *and* if the class's equals() or hashCode() methods (which are called from
within HashSet) were synchronized *and* if at some point you called a method on the set
without first synchronizing on "this".

Thankyou Michael, that clarifies the issue.