Question reg. holding Locks during invocation of Lambdas

[Doug Lea]

On 06/18/13 05:11, Gernot Neppert wrote:
> Hi all,
>
> I noticed that ConcurrentHashMap.computeIfAbsent() invokes the
> user-supplied callback while being in a synchronized block.
>
> One of the rules I stick to in programming is "Don't invoke a user-supplied
> callback from a library while holding a Lock".
> (because it can lead to nasty deadlocks).

Yes, this is exactly why we did not originally provide
such a method in ConcurrentMap. But now we know that this
was the wrong choice. Empirical reports imply that many more
bugs are caused by people not correctly maintaining atomicity
when they don't have this method than would be caused by
supporting it and thus making it easier to encounter deadlock.

(See for example Shacham et al's OOPSLA 2011 paper
at http://www.cs.tau.ac.il/~ohads/)

-Doug


>
> If I understand it correctly, the purpose of computeIfAbsent() is delaying
> the invocation of a possibly lengthy factory-expression until it has been
> determined that a mapping for the key is not already present.
>
> Thus, shouldn't the callback better be called outside any held Locks?
>
> For those interested in  more details:
> A while back I wrote a class "CacheMap" that serves as a
> drop-in-replacement for java.util.Map. Its get(Object key) method invokes a
> uses-supplied callback if a mapping is not already present.
> In the implementation I went to great lengths to ensure that no Locks are
> held while invoking the callback while at the same time guaranteeing
> consistency in a multi-threaded scenario.
> (The Map-wide Lock is only held for a short duration to atomically check
> for the key and put a temporary "factory" object in the internal Map. This
> factory object then uses wait/notify to ensure that subsequent threads
> requiring the same key will block until the callback has been invoked).
> So I know it's possible to do it "correctly". Now I'd be reluctant to
> replace my "CacheMap" with JDK's "ConcurrentHashMap" because of the
> possible deadlocks waiting to happen.
>