RFR:8047290:Ensure consistent safepoint checking in MutexLockerEx

[Max Ockner]

David,
Thanks for the comments.  I have replied individually to each comment below:

On 10/15/2014 8:54 PM, David Holmes wrote:
> Hi Max,
>
> This is looking good.
>
> A few high-level initial comments:
>
> I think SafepointAllowed should be SafepointCheckNeeded

Sure, I can change this.

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

Good point.  This didn't occur to me when I first made the change, but I 
will move the check into the mutex/monitor locking 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.

Not sure how at the moment, but I can file another bug for this.

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

I could not verify that any were being used correctly/incorrectly, and 
I'm afraid that I would break the code if I forced these locks to 
safepoint check consistently.


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