Re: [infinispan-dev] race condition in AsyncStore

On Oct 28, 2009, at 7:02 PM, Manik Surtani wrote: > Holding on to the lock until the put happens is not good, it will > block user threads from enqueueing changes. true > > 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(); > } > That would serialize the access to the cached modifications for one thread at a time, so multuple AsyncProcessors would not be needed.

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.

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