RFR 8213150: Add verification for locking by VMThread

Robbin Ehn robbin.ehn at oracle.com
Mon Sep 23 13:37:35 UTC 2019


Hi Coleen,

src/hotspot/share/oops/constantPool.cpp
  561     if (access_check && k != NULL) {

access_check must be same as !SafepointSynchronize::is_at_safepoint(), correct?
So either change access_check to !SafepointSynchronize::is_at_safepoint() or 
assert it.

src/hotspot/share/runtime/mutex.cpp
   49   if (do_safepoint_check && thread->is_active_Java_thread()) {
   50     assert(((JavaThread *)thread)->thread_state() == _thread_in_vm || 
rank() == Mutex::special,
   51            "wrong thread state for using safepoint checking locks");
   52
   53     if (rank() != Mutex::special) {
   54       thread->check_for_valid_safepoint_state();
   55     }
   56   }

This is not correct, you should be using _mutex->rank() <= Mutex::special.
But there is no point in checking it here, just check this in the constructor 
that special and below are never locks. (since it is a never lock we should not 
do_safepoint_check thus state check is not needed)

do_safepoint_check is checked four times (once via not_allowed), can you please 
re-write this method, or better yet remove the bool input.
The bool is hardcoded so can you just create two methods, with one helper method 
for the common checks. (yes there will be more code, but readable :) )

Thanks, Robbin

On 9/20/19 11:36 PM, coleen.phillimore at oracle.com wrote:
> Summary: extend verification for all locking not just VMOperations, and fix CLDG 
> lock to not be taken by VM thread.
> 
> See bug comments for more details about this change.
> 
> Tested with tier1 all Oracle platforms, tier2-8 linux-x64-debug.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/2019/8213150.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8213150
> 
> Thanks,
> Coleen


More information about the hotspot-runtime-dev mailing list