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?
