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