RFR(S): 8039458: Ensure consistency of Monitor/Mutex lock acquisitions in relation to safepoint protocol

Markus Grönlund markus.gronlund at oracle.com
Mon Apr 14 08:42:47 UTC 2014

Many thanks for taking a look David.

I will add another accessor which will allow for some easier read:

+ if (acquired_with_safepoint_check()) {
+        if (locks->acquired_with_no_safepoint_check()) {

I will also ensure to get some larger scale testing for detecting any existing potentially broken behaviour.

Thanks again

-----Original Message-----
From: David Holmes 
Sent: den 10 april 2014 12:00
To: Markus Grönlund; hotspot-runtime-dev; serviceability-dev at openjdk.net; hotspot-compiler-dev at openjdk.java.net; hotspot-gc-dev at openjdk.java.net
Subject: Re: RFR(S): 8039458: Ensure consistency of Monitor/Mutex lock acquisitions in relation to safepoint protocol

Hi Markus,

Not sure about the need to set _acquired_with_no_safepoint_check = false in unlock, but otherwise this looks okay to me. You only need to check the most recently acquired lock, because when you acquired it you would have checked the previously acquired lock and so forth.

The double negation, !_acquired_with_no_safepoint_check, does make this a little hard to think about. :)

I think you may need some additional test coverage to see if this is going to expose existing potentially broken behaviour.


On 9/04/2014 10:04 PM, Markus Grönlund wrote:
> Greetings,
> Kindly asking for reviews for the following bug fix/enhancement
> Webrev: http://cr.openjdk.java.net/~mgronlun/8039458/webrev01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8039458
> Problem:
> VM code acquire Monitor/Mutex locks by respecting the correct lock 
> order, which is enforced in debug builds (asserts). This is great.
> However, you could be taking locks in the correct order and everything 
> might work just fine during development and testing (even product) but 
> depending on runtime circumstances (load/concurrency), the way the
> lock(s) was acquired, i.e. the lock acquisition /mode/, can lead to 
> very hard to debug problems (hangs/deadlocks). This is primarily 
> related to the lock acquisition mode option 
> "Mutex::_no_safepoint_check_flag" by which you inform your intent to 
> respect or not-respect the safepoint protocol during lock acquisition/ownership.
> If a lock has already been acquired by a thread by passing the 
> Mutex::_no_safepoint_check_flag to circumvent the safepoint protocol, 
> the thread must *not* be allowed to attempt taking yet another lock on 
> which it *might* block ( a lock which was not acquired by passing 
> Mutex::_no_safepoint_check_flag in its acquisition operation), as such 
> a lock would be participating in the safepoint protocol.
> Today, doing this mistake will work just fine _/if the lock is 
> currently uncontended_ /which it normally is in development/testing 
> situations*.* However, once the lock then becomes contended (highly 
> volatile pressured/concurrent product deployment) you might get the 
> worst kind of issues to debug (hangs/deadlocks).
> Naturally and as always, you have to very careful when you are taking 
> locks using the Mutex::_no_safepoint_check_flag -however it can be 
> very hard to determine what other code you can safely call once you 
> are the owner of a Mutex::_no_safepoint_check_flag:ed lock  - and  
> today there is nothing but the developers attention to details that 
> will find/help stay clear of this. I think we can do better here in 
> enforcing some invariants in code for preempting potential deadlock prone situations.
> Description:
> Add debug assertions that enforce correct usages of the 
> Mutex::_no_safepoint_check_flag when taking multiple locks.
> This suggestion leverages much of the existing code targeting 
> enforcement of lock ordering- this is a simple additive change to also 
> add invariant checking on "lock acquisition mode".
> Do note this primarily targets Java Threads running in the VM.
> Also included code comment:
> /*
>   * Ensure consistency of Monitor/Mutex lock acquisitions
>   * for Java Threads running inside the VM.
>   *
>   * If a thread has already acquired lock(s) using
>   * "Mutex::_no_safepoint_check_flag" (effectively going outside the
>   * safepoint protocol), the thread should be disallowed to acquire any
>   * additional lock which _is_ participating in the safepoint protocol.
>   *
>   * If a "safepoint protocol aware" lock is contended, and the thread 
> entering
>   * is unable to "fast acquire" the lock using cas/try_spin,
>   * it will need to block/park. Blocking on a contended lock involves a
>   * state transition and a potential SafepointSynchronize::block() call.
>   * Transitioning to a blocked state still holding 
> "Mutex::_no_safepoint_check_flag"
>   * acquired locks is allowed, but is *very* deadlock prone.
>   *
>   * The effect of allowing this state to happen without checking is subtle
>   * and hard to debug since a problem might only appear under heavy 
> load and
>   * only in situations with increased concurrency levels (product builds).
> *
>   */
> Testing:
> Artificial code changes to MutexLocker and 
> Mutex::_no_safepoint_check_flag on certain locks for detecting 
> potential deadlock situations.
> Speccjbb2005
> Kitchensink
> Thank you
> Markus

More information about the hotspot-runtime-dev mailing list