Holding on to the lock until the put happens is not good, it will block user threads from enqueueing changes.

Perhaps what you need is an additional lock, an AsyncProcessor lock that AsyncProcessor threads need to acquire. E.g., AsyncProcessor:254 changes from: try { run0(); } catch (InterruptedException e) { break; } to something like: try { acquireLock(asyncProcessorLock); run0(); } catch (InterruptedException e) { break; } finally { asyncProcessorLock.unlock(); }

On 28 Oct 2009, at 16:51, Mircea Markus wrote: > Hi Galder, > > This is following an email sent this morning on an infinit loop in > AsyncTest. I've found what was[1] causing the infinite loop. Here it > is (see comments in the code): > > Let's say we do put(k1, v1) and put(k1, v2) in this order > > > void run0() throws InterruptedException { > if (trace) log.trace("Checking for modifications"); > boolean unlock = false; > try { > acquireLock(write); > unlock = true; > swap = state; > state = newStateMap(); > } finally { > if (unlock) write.unlock(); > } > //thread ONE comes here and is processing (k1, v1); it releases the > write lock. Immediately, thread TWO acquires write lock, and takes > (k1,v2) from the store into its swap. > // the race condition happens when thread TWO executes (i.e. > persists) > before thread one, and as a result in the store the final value > persisted is v1 (not v2). > // This is known to be an async (i.e. best effort store), so this > situation might be considered as normal behavior. Still, I think this > induces unwanted errors/behavior. > // A possible solution is to only release the WL after put(swap). > > > if (trace) log.trace("About to insert the swap: " + swap); > int size = swap.size(); > if (size == 0) > awaitNotEmpty(); > else > decrementAndGet(size); > > if (trace) log.trace("Calling put(List) with {0} > modifications", size); > put(swap); > } > > Cheers, > Mircea > > > > [1] I say 'was' because I modified the test to fail (intermittently) > rather than hang > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 28 Oct 2009, at 17:12, Mircea Markus wrote:

> I think the way to go is: > > > void run0() throws InterruptedException { > if (trace) log.trace("Checking for modifications"); > boolean unlock = false; > try { > acquireLock(write); > unlock = true; > swap = state; > //for each key in the swap acquire a WL. Do this with 0 ms acquisition > timeout to make sure that the put does not block. That would mean a lot of locks. :) But yeah that approach would work. Perhaps look at AbstractPerEntryLockContainer to look at how such locks are created lazily and destroyed when no longer in use.

> state = newStateMap(); > > > } finally { > if (unlock) write.unlock(); > } > > int size = swap.size(); > if (size == 0) > awaitNotEmpty(); > else > decrementAndGet(size); > > if (trace) log.trace("Calling put(List) with {0} > modifications", size); > put(swap); > > //for each key in the swap release WL > } > > This way we would make sure that the access *per key* is serialized. > > >> >> >> On 28 Oct 2009, at 16:51, Mircea Markus wrote: >> >>> Hi Galder, >>> >>> This is following an email sent this morning on an infinit loop in >>> AsyncTest. I've found what was[1] causing the infinite loop. Here it >>> is (see comments in the code): >>> >>> Let's say we do put(k1, v1) and put(k1, v2) in this order >>> >>> >>> void run0() throws InterruptedException { >>> if (trace) log.trace("Checking for modifications"); >>> boolean unlock = false; >>> try { >>> acquireLock(write); >>> unlock = true; >>> swap = state; >>> state = newStateMap(); >>> } finally { >>> if (unlock) write.unlock(); >>> } >>> //thread ONE comes here and is processing (k1, v1); it releases the >>> write lock. Immediately, thread TWO acquires write lock, and takes >>> (k1,v2) from the store into its swap. >>> // the race condition happens when thread TWO executes (i.e. >>> persists) >>> before thread one, and as a result in the store the final value >>> persisted is v1 (not v2). >>> // This is known to be an async (i.e. best effort store), so this >>> situation might be considered as normal behavior. Still, I think >>> this >>> induces unwanted errors/behavior. >>> // A possible solution is to only release the WL after put(swap). >>> >>> >>> if (trace) log.trace("About to insert the swap: " + swap); >>> int size = swap.size(); >>> if (size == 0) >>> awaitNotEmpty(); >>> else >>> decrementAndGet(size); >>> >>> if (trace) log.trace("Calling put(List) with {0} >>> modifications", size); >>> put(swap); >>> } >>> >>> Cheers, >>> Mircea >>> >>> >>> >>> [1] I say 'was' because I modified the test to fail (intermittently) >>> rather than hang >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> -- >> Manik Surtani >> manik(a)jboss.org >> Lead, Infinispan >> Lead, JBoss Cache >> http://www.infinispan.org >> http://www.jbosscache.org >> >> >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev