[icedtea-web] RFC: Patch to use wait() when waiting for conditions to become true

[Deepak Bhole]

* Dr Andrew John Hughes <ahughes at redhat.com> [2011-04-14 18:22]:
> On 16:58 Thu 14 Apr     , Deepak Bhole wrote:
> > * Dr Andrew John Hughes <ahughes at redhat.com> [2011-04-14 16:32]:
> > > 
> > > CHMs aren't locked on gets either.  They do have visibility guarantees.
> > > 
> > > I'm a little worried about using the monitor of an object that we know does its
> > > own internal locking.  If we're keeping CHMs, we should use our own locks in this
> > > class so we don't interfere.  It doesn't matter what objects are used to synchronise
> > > on, just that we are consistent.
> > >
> > 
> > Denis brought this up too. 
> 
> Yes, though I did first in my initial comments.
> 
> > Maybe I am missing something -- even if CHM
> > does its own locking, wouldn't the caller thread still own the monitor,
> > thus making it a non-issue? We'd just be locking it earlier. If outside
> > locking is a concern, that would make CHM very dangerous to use in any
> > sort of multi-threaded environment that might lock it before an op.
> > 
> 
> Client-side locking is always dangerous unless you know the locking
> policy of the class.  With Vector, Hashtable and
> Collections.synchronizedX it is documented that you lock on the
> collection.  CHM explictly says that you can't lock the entire map.
> This is one of the reasons it provides putIfAbsent, etc.  You can't do
> it externally.  How do you know that another thread hasn't obtained
> the monitor due to a call to one of the CHM methods?
>

I think CHMs documentation is a bit misleading there. What that implies
is that none of the operations exclusively lock (unlike
Collections.synchronizedX which synchronizes each method around a single
mutex). Locking on the object itself is still possible (and the
generated bytecode seems to agree).
 
> I think it is safer and clearer to use separate locks as we aren't
> even using them to lock the collection contents but as signals.  Even
> if you can guarantee that using the monitor of the collections won't
> cause issues, simplifying the code is generally a good idea to protect
> against mistakes.  On that subject, this whole class probably needs a
> rewrite.
> 

Fair enough. I won't argue from an aesthetics POV, especially since there
is already a patch :) Checking it out now...

Cheers,
Deepak