Question reg. holding Locks during invocation of Lambdas

[David Holmes]

Hi Gernot,

On 18/06/2013 7:11 PM, 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).
>
> 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?

The contract for computeIfAbsent requires atomicity and a 
compute-at-most-once guarantee, hence the locking is needed. The compute 
function should be simple and self-contained to minimise potential for 
interactions that might lead to deadlocks, as per the docs:

      *                                                    The entire
      * method invocation is performed atomically, so the function is
      * applied at most once per key.  Some attempted update operations
      * on this map by other threads may be blocked while computation
      * is in progress, so the computation should be short and simple,
      * and must not attempt to update any other mappings of this map.

Sensible compute functions are unlikely to lead to deadlocks.

David
-----

> 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.
>