RFR: 8166197: assert(RelaxAssert || w != Thread::current()->_MutexEvent) failed: invariant

[Carsten Varming]

Dear David,

In line 590 "Pass onDeck to w". I don't understand this part of the
comment. Should it say something like "Pass ownership of _OnDeck and
_EntryList to w".

In line 532 there is a read of _OnDeck and in line 553 there is a read of
_EntryList. Why is it safe not to do a load_acquire of _OnDeck in line 532?

Carsten


On Wed, Oct 12, 2016 at 1:08 AM, David Holmes <david.holmes at oracle.com>
wrote:

> Bug: https://bugs.openjdk.java.net/browse/JDK-8166197
> webrev: http://cr.openjdk.java.net/~dholmes/8166197/webrev/
>
> In IUnlock we have the following succession code to wakeup the "onDeck"
> thread:
>
>  ParkEvent * List = _EntryList;
>   if (List != NULL) {
>     // Transfer the head of the EntryList to the OnDeck position.
>     // Once OnDeck, a thread stays OnDeck until it acquires the lock.
>     // For a given lock there is at most OnDeck thread at any one instant.
>    WakeOne:
>     assert(List == _EntryList, "invariant");
>     ParkEvent * const w = List;
>     assert(RelaxAssert || w != Thread::current()->_MutexEvent,
> "invariant");
>     _EntryList = w->ListNext;
>     // as a diagnostic measure consider setting w->_ListNext = BAD
>     assert(UNS(_OnDeck) == _LBIT, "invariant");
>     _OnDeck = w;  // pass OnDeck to w.
>
> It is critical that the update to _EntryList happens before we set
> _OnDeck, as as soon as _OnDeck is set the selected thread (which need not
> yet have parked) can acquire the mutex, complete its critical section and
> proceed to unlock the mutex, and so execute IUnlock in parallel with the
> original thread. If the write to _EntryList has not yet happened that
> second thread finds itself still at the head of _EntryList and so the
> assert fires. If the write to _EntryList happens after the load "List =
> _EntryList", then the first assert can also fire.
>
> Preferred fix today is to use OrderAccess::release_store(&_OnDeck, w)
> with a matching load_acquire(&_OnDeck) in the ILock code:
>
>   while (_OnDeck != ESelf) {
>     ParkCommon(ESelf, 0);
>   }
>
> and corresponding "raw" lock code. Also fixed a couple of typos.
>
> Thanks,
> David
>