Manik, how about adding a reference count to the lock entry? If there is a waiter on the lock, the reverence count will be > 0 and the owner won't remove the key on unlock.


On Tue, Oct 16, 2012 at 3:43 AM, Manik Surtani <manik@jboss.org> wrote:
Hmm, that actually might just do the trick.  Thanks!

On 15 Oct 2012, at 17:46, Jason Greene <jason.greene@redhat.com> wrote:

I think what you are looking for is this:

http://gee.cs.oswego.edu/dl/jsr166/dist/jsr166edocs/jsr166e/ConcurrentHashMapV8.html#computeIfAbsent(K, jsr166e.ConcurrentHashMapV8.Fun)

On Oct 15, 2012, at 11:23 AM, Manik Surtani <manik@jboss.org> wrote:

Guys, after investigating https://issues.jboss.org/browse/ISPN-2381 and https://github.com/infinispan/infinispan/pull/1382, I've discovered a pretty nasty race condition in our per-entry lock containers (whether OwnableReentrantLocks or JDK locks for non-transactional caches).

The problem is that we maintain a lock map, and any given request can acquire a lock, if a lock doesn't exist for a given key, create the lock and acquire it, and when done, release the lock and remove it from the lock map.  There's lots of room for races to occur.  The current impl uses a ConcurrentMap, where concurrent operations on the map are used to make sure locks are not overwritten.  But still, since the process of creating, acquiring and adding the lock to the lock map needs to be atomic, and not just atomic but also safe with regards to competing threads (say, an old owner) releasing the lock and removing it from the map (also atomic), I think a concurrent map isn't good enough anymore.

The sledgehammer approach is to synchronise on this map for these two operations, but that causes all sorts of suckage.  Ideally, I'd just hold on to the segment lock for the duration of these operations, but these aren't exposed.  Extending CHM to expose methods like acquireLockAndGet() and unlockAndRemove() would work perfectly, but again a lot of CHM internals are private or package protected.

So my options are: completely re-implement a CHM-like structure, like we've done for BCHM, or perhaps think of a new, specialised structure to contain locks.  In terms of contract, I just need a fast way to look up a value under given a key, efficient put and remove as well.  It should be thread-safe (naturally), and allow for an atomic operation (like "get, do work, put").

Any interesting new data structures on peoples' minds?

Cheers
Manik
--
Manik Surtani
manik@jboss.org
twitter.com/maniksurtani

Platform Architect, JBoss Data Grid
http://red.ht/data-grid



--
Manik Surtani

Platform Architect, JBoss Data Grid


_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev