RFR:8047290:Ensure consistent safepoint checking in MutexLockerEx

[David Holmes]

Hi Max,

This is looking good.

A few high-level initial comments:

I think SafepointAllowed should be SafepointCheckNeeded

Why are the checks in MutexLocker when the state is maintained in the 
mutex itself and the mutex/monitor has lock_without_safepoint, and 
wait(<safepoint checking flag>) ? I would have expected to see the check 
in the mutex/monitor methods.

Checking consistent usage of the _no_safepoint_check_flag is good. But 
another part of this is that a monitor/mutex that never checks for 
safepoints should never be held when a thread blocks at a safepoint - is 
there some way to easily check that? I was surprised how many locks are 
actually not checking for safepoints.

Did you find any cases where the mutex/monitor was being used 
inconsistently and incorrectly?

Did you analyse the "sometimes" cases to see if they were safe? (Aside: 
just for fun check out what happens if you lock the Threads_lock with a 
safepoint check and a safepoint has been requested :) ).

Cheers,
David

On 16/10/2014 4:04 AM, Max Ockner wrote:
> Hi all,
>
> I am a new member of the Hotspot runtime team in Burlington, MA.
> Please review my first fix related to safepoint checking.
>
> Summary: MutexLockerEx can either acquire a lock with or without a
> safepoint check.
> In some cases, a particular lock must either safepoint check always or
> never to avoid deadlocking.
> Some other locks have semantics which allow them to avoid deadlocks
> despite having a safepoint check only some of the time.
> All locks that are OK having inconsistent safepoint checks have been
> marked. All locks that should never safepoint check and all locks that
> should always safepoint check have also been marked.
> When a MutexLockerEx acquires a lock with or without a safepoint check,
> the lock's safepointAllowed marker is checked to ensure consistent
> safepoint checking.
>
> Webrev: http://oklahoma.us.oracle.com/~mockner/webrev/8047290/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8047290
>
> Tested with:
>      jprt "-testset hotspot"
>      jtreg hotspot
>      vm.quick.testlist
>
> Whitebox tests:
> test/runtime/Safepoint/AssertSafepointCheckConsistency1.java: Test
> expects Assert ("This lock should always have a safepoint check")
> test/runtime/Safepoint/AssertSafepointCheckConsistency2.java: Test
> expects Assert ("This lock should never have a safepoint check")
> test/runtime/Safepoint/AssertSafepointCheckConsistency3.java: code
> should not assert. (Lock is properly acquired with no safepoint check)
> test/runtime/Safepoint/AssertSafepointCheckConsistency4.java: code
> should not assert. (Lock is properly acquired with safepoint check)
>
> Thanks,
> Max
>