[infinispan-dev] race condition in AsyncStore

[Manik Surtani]

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

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

Yeah, true.

> 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 at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>> --
>> Manik Surtani
>> manik at jboss.org
>> Lead, Infinispan
>> Lead, JBoss Cache
>> http://www.infinispan.org
>> http://www.jbosscache.org
>>
>>
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
manik at jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org