Ivan, i found that mentioned problem correctly highlighted, if we still not
step on this rake, its probably due to some locks above, anyway it would be
correct to fix this part. Are you fill the ticket ?
>
>
>------- Forwarded message -------
>From: "Ivan Daschinsky" < [email protected] >
>To: [email protected]
>Cc:
>Subject: Re: Possible concurrency bug in GridCacheLockImpl
>Date: Fri, 03 Apr 2020 17:07:22 +0300
>
>Sorry, I meant not GridCacheLockImpl but CacheLockImpl.
>
>пт, 3 апр. 2020 г. в 17:04, Ivan Daschinskiy < [email protected] >:
>
>> Folks,
>>
>> Lurking through code, it seems I found some concurrency issue in subj.
>>
>> * This class contains two fields, volatile Thread lockedThread and
>> non-volatile int cntr (used for reentrancy)
>>
>> * private method incrementLockingCount() is called inside lock(). In this
>> method we perform volatile read in assert
>> (assert (lockedThread == null && cntr == 0) || (lockedThread ==
>> Thread.currentThread() && cntr > 0) then increment cntr;
>> * In method unlock(), we firstly decrement cntr and after that if cntr
>> equals to 0, performs volatile write to lockedThread.
>>
>> Suppose execution when asserts are enabled.
>>
>> T1 | T2
>>
>> -------------------------------------------------------------------------------
>> cntr = cntr - 1 (cntr == 0) |
>> lockedThread = null |
>> | lockedThread ==
>> null
>> && cntr == 0
>> | cntr = cntr + 1
>> (cntr
>> == 1)
>>
>>
>> There is a happens-before edge and we have strong guarantee that
>> reentrancy will works and cntr will definitely equals to 1;
>>
>> But if assertions are disabled, something can go wrong.
>>
>> Moreover, if assertions are disabled, we can allow other thread do obtain
>> lock even if our thread holds it.
>>
>> I think that this should be fixed, for example we can throw
>> IllegalStateException, as in unlock() method.
>>
>> WDYT?
>>
>>
