8047212: runtime/ParallelClassLoading/bootstrap/random/inner-complex assert(ObjectSynchronizer::verify_objmon_isinpool(inf)) failed: monitor is invalid jdk8u252-b04

Sat, 24 Oct 2015 15:44:08 -0700

author
fyang
date
Sat, 24 Oct 2015 15:44:08 -0700
changeset 9838
ff1c3c1867b5
parent 9837
e517ff39c40d
child 9839
e314de338c65

8047212: runtime/ParallelClassLoading/bootstrap/random/inner-complex assert(ObjectSynchronizer::verify_objmon_isinpool(inf)) failed: monitor is invalid
Summary: Fix race between ObjectMonitor alloc and verification code; teach SA about "static pointer volatile" fields.
Reviewed-by: andrew

src/share/vm/runtime/synchronizer.cpp file | annotate | diff | comparison | revisions
src/share/vm/runtime/synchronizer.hpp file | annotate | diff | comparison | revisions
src/share/vm/runtime/vmStructs.cpp file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/vm/runtime/synchronizer.cpp	Wed Nov 04 10:12:37 2015 -0800
     1.2 +++ b/src/share/vm/runtime/synchronizer.cpp	Sat Oct 24 15:44:08 2015 -0700
     1.3 @@ -149,7 +149,7 @@
     1.4  #define NINFLATIONLOCKS 256
     1.5  static volatile intptr_t InflationLocks [NINFLATIONLOCKS] ;
     1.6  
     1.7 -ObjectMonitor * ObjectSynchronizer::gBlockList = NULL ;
     1.8 +ObjectMonitor * volatile ObjectSynchronizer::gBlockList = NULL;
     1.9  ObjectMonitor * volatile ObjectSynchronizer::gFreeList  = NULL ;
    1.10  ObjectMonitor * volatile ObjectSynchronizer::gOmInUseList  = NULL ;
    1.11  int ObjectSynchronizer::gOmInUseCount = 0;
    1.12 @@ -830,18 +830,18 @@
    1.13  // Visitors ...
    1.14  
    1.15  void ObjectSynchronizer::monitors_iterate(MonitorClosure* closure) {
    1.16 -  ObjectMonitor* block = gBlockList;
    1.17 -  ObjectMonitor* mid;
    1.18 -  while (block) {
    1.19 +  ObjectMonitor* block =
    1.20 +    (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
    1.21 +  while (block != NULL) {
    1.22      assert(block->object() == CHAINMARKER, "must be a block header");
    1.23      for (int i = _BLOCKSIZE - 1; i > 0; i--) {
    1.24 -      mid = block + i;
    1.25 -      oop object = (oop) mid->object();
    1.26 +      ObjectMonitor* mid = (ObjectMonitor *)(block + i);
    1.27 +      oop object = (oop)mid->object();
    1.28        if (object != NULL) {
    1.29          closure->do_monitor(mid);
    1.30        }
    1.31      }
    1.32 -    block = (ObjectMonitor*) block->FreeNext;
    1.33 +    block = (ObjectMonitor*)block->FreeNext;
    1.34    }
    1.35  }
    1.36  
    1.37 @@ -856,7 +856,9 @@
    1.38  
    1.39  void ObjectSynchronizer::oops_do(OopClosure* f) {
    1.40    assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
    1.41 -  for (ObjectMonitor* block = gBlockList; block != NULL; block = next(block)) {
    1.42 +  ObjectMonitor* block =
    1.43 +    (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
    1.44 +  for (; block != NULL; block = (ObjectMonitor *)next(block)) {
    1.45      assert(block->object() == CHAINMARKER, "must be a block header");
    1.46      for (int i = 1; i < _BLOCKSIZE; i++) {
    1.47        ObjectMonitor* mid = &block[i];
    1.48 @@ -1059,7 +1061,9 @@
    1.49          // The very first objectMonitor in a block is reserved and dedicated.
    1.50          // It serves as blocklist "next" linkage.
    1.51          temp[0].FreeNext = gBlockList;
    1.52 -        gBlockList = temp;
    1.53 +        // There are lock-free uses of gBlockList so make sure that
    1.54 +        // the previous stores happen before we update gBlockList.
    1.55 +        OrderAccess::release_store_ptr(&gBlockList, temp);
    1.56  
    1.57          // Add the new string of objectMonitors to the global free list
    1.58          temp[_BLOCKSIZE - 1].FreeNext = gFreeList ;
    1.59 @@ -1536,29 +1540,33 @@
    1.60       nInuse += gOmInUseCount;
    1.61      }
    1.62  
    1.63 -  } else for (ObjectMonitor* block = gBlockList; block != NULL; block = next(block)) {
    1.64 -  // Iterate over all extant monitors - Scavenge all idle monitors.
    1.65 -    assert(block->object() == CHAINMARKER, "must be a block header");
    1.66 -    nInCirculation += _BLOCKSIZE ;
    1.67 -    for (int i = 1 ; i < _BLOCKSIZE; i++) {
    1.68 -      ObjectMonitor* mid = &block[i];
    1.69 -      oop obj = (oop) mid->object();
    1.70 +  } else {
    1.71 +    ObjectMonitor* block =
    1.72 +      (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
    1.73 +    for (; block != NULL; block = (ObjectMonitor*)next(block)) {
    1.74 +      // Iterate over all extant monitors - Scavenge all idle monitors.
    1.75 +      assert(block->object() == CHAINMARKER, "must be a block header");
    1.76 +      nInCirculation += _BLOCKSIZE;
    1.77 +      for (int i = 1; i < _BLOCKSIZE; i++) {
    1.78 +        ObjectMonitor* mid = (ObjectMonitor*)&block[i];
    1.79 +        oop obj = (oop)mid->object();
    1.80  
    1.81 -      if (obj == NULL) {
    1.82 -        // The monitor is not associated with an object.
    1.83 -        // The monitor should either be a thread-specific private
    1.84 -        // free list or the global free list.
    1.85 -        // obj == NULL IMPLIES mid->is_busy() == 0
    1.86 -        guarantee (!mid->is_busy(), "invariant") ;
    1.87 -        continue ;
    1.88 -      }
    1.89 -      deflated = deflate_monitor(mid, obj, &FreeHead, &FreeTail);
    1.90 +        if (obj == NULL) {
    1.91 +          // The monitor is not associated with an object.
    1.92 +          // The monitor should either be a thread-specific private
    1.93 +          // free list or the global free list.
    1.94 +          // obj == NULL IMPLIES mid->is_busy() == 0
    1.95 +          guarantee(!mid->is_busy(), "invariant");
    1.96 +          continue;
    1.97 +        }
    1.98 +        deflated = deflate_monitor(mid, obj, &FreeHead, &FreeTail);
    1.99  
   1.100 -      if (deflated) {
   1.101 -        mid->FreeNext = NULL ;
   1.102 -        nScavenged ++ ;
   1.103 -      } else {
   1.104 -        nInuse ++;
   1.105 +        if (deflated) {
   1.106 +          mid->FreeNext = NULL;
   1.107 +          nScavenged++;
   1.108 +        } else {
   1.109 +          nInuse++;
   1.110 +        }
   1.111        }
   1.112      }
   1.113    }
   1.114 @@ -1693,13 +1701,13 @@
   1.115  
   1.116  // Verify all monitors in the monitor cache, the verification is weak.
   1.117  void ObjectSynchronizer::verify() {
   1.118 -  ObjectMonitor* block = gBlockList;
   1.119 -  ObjectMonitor* mid;
   1.120 -  while (block) {
   1.121 +  ObjectMonitor* block =
   1.122 +    (ObjectMonitor *)OrderAccess::load_ptr_acquire(&gBlockList);
   1.123 +  while (block != NULL) {
   1.124      assert(block->object() == CHAINMARKER, "must be a block header");
   1.125      for (int i = 1; i < _BLOCKSIZE; i++) {
   1.126 -      mid = block + i;
   1.127 -      oop object = (oop) mid->object();
   1.128 +      ObjectMonitor* mid = (ObjectMonitor *)(block + i);
   1.129 +      oop object = (oop)mid->object();
   1.130        if (object != NULL) {
   1.131          mid->verify();
   1.132        }
   1.133 @@ -1713,18 +1721,18 @@
   1.134  // the list of extant blocks without taking a lock.
   1.135  
   1.136  int ObjectSynchronizer::verify_objmon_isinpool(ObjectMonitor *monitor) {
   1.137 -  ObjectMonitor* block = gBlockList;
   1.138 -
   1.139 -  while (block) {
   1.140 +  ObjectMonitor* block =
   1.141 +    (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
   1.142 +  while (block != NULL) {
   1.143      assert(block->object() == CHAINMARKER, "must be a block header");
   1.144      if (monitor > &block[0] && monitor < &block[_BLOCKSIZE]) {
   1.145 -      address mon = (address) monitor;
   1.146 -      address blk = (address) block;
   1.147 +      address mon = (address)monitor;
   1.148 +      address blk = (address)block;
   1.149        size_t diff = mon - blk;
   1.150 -      assert((diff % sizeof(ObjectMonitor)) == 0, "check");
   1.151 +      assert((diff % sizeof(ObjectMonitor)) == 0, "must be aligned");
   1.152        return 1;
   1.153      }
   1.154 -    block = (ObjectMonitor*) block->FreeNext;
   1.155 +    block = (ObjectMonitor*)block->FreeNext;
   1.156    }
   1.157    return 0;
   1.158  }
     2.1 --- a/src/share/vm/runtime/synchronizer.hpp	Wed Nov 04 10:12:37 2015 -0800
     2.2 +++ b/src/share/vm/runtime/synchronizer.hpp	Sat Oct 24 15:44:08 2015 -0700
     2.3 @@ -131,7 +131,7 @@
     2.4  
     2.5   private:
     2.6    enum { _BLOCKSIZE = 128 };
     2.7 -  static ObjectMonitor* gBlockList;
     2.8 +  static ObjectMonitor * volatile gBlockList;
     2.9    static ObjectMonitor * volatile gFreeList;
    2.10    static ObjectMonitor * volatile gOmInUseList; // for moribund thread, so monitors they inflated still get scanned
    2.11    static int gOmInUseCount;
     3.1 --- a/src/share/vm/runtime/vmStructs.cpp	Wed Nov 04 10:12:37 2015 -0800
     3.2 +++ b/src/share/vm/runtime/vmStructs.cpp	Sat Oct 24 15:44:08 2015 -0700
     3.3 @@ -257,6 +257,7 @@
     3.4  
     3.5  #define VM_STRUCTS(nonstatic_field, \
     3.6                     static_field, \
     3.7 +                   static_ptr_volatile_field, \
     3.8                     unchecked_nonstatic_field, \
     3.9                     volatile_nonstatic_field, \
    3.10                     nonproduct_nonstatic_field, \
    3.11 @@ -1082,7 +1083,7 @@
    3.12    volatile_nonstatic_field(BasicLock,          _displaced_header,                             markOop)                               \
    3.13    nonstatic_field(BasicObjectLock,             _lock,                                         BasicLock)                             \
    3.14    nonstatic_field(BasicObjectLock,             _obj,                                          oop)                                   \
    3.15 -  static_field(ObjectSynchronizer,             gBlockList,                                    ObjectMonitor*)                        \
    3.16 +  static_ptr_volatile_field(ObjectSynchronizer,gBlockList,                                    ObjectMonitor*)                        \
    3.17                                                                                                                                       \
    3.18    /*********************/                                                                                                            \
    3.19    /* Matcher (C2 only) */                                                                                                            \
    3.20 @@ -2667,6 +2668,11 @@
    3.21  #define GENERATE_STATIC_VM_STRUCT_ENTRY(typeName, fieldName, type)                 \
    3.22   { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, &typeName::fieldName },
    3.23  
    3.24 +// This macro generates a VMStructEntry line for a static pointer volatile field,
    3.25 +// e.g.: "static ObjectMonitor * volatile gBlockList;"
    3.26 +#define GENERATE_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY(typeName, fieldName, type)    \
    3.27 + { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, (void*)&typeName::fieldName },
    3.28 +
    3.29  // This macro generates a VMStructEntry line for an unchecked
    3.30  // nonstatic field, in which the size of the type is also specified.
    3.31  // The type string is given as NULL, indicating an "opaque" type.
    3.32 @@ -2692,10 +2698,15 @@
    3.33  #define CHECK_VOLATILE_NONSTATIC_VM_STRUCT_ENTRY(typeName, fieldName, type)        \
    3.34   {typedef type dummyvtype; typeName *dummyObj = NULL; volatile dummyvtype* dummy = &dummyObj->fieldName; }
    3.35  
    3.36 -// This macro checks the type of a VMStructEntry by comparing pointer types
    3.37 +// This macro checks the type of a static VMStructEntry by comparing pointer types
    3.38  #define CHECK_STATIC_VM_STRUCT_ENTRY(typeName, fieldName, type)                    \
    3.39   {type* dummy = &typeName::fieldName; }
    3.40  
    3.41 +// This macro checks the type of a static pointer volatile VMStructEntry by comparing pointer types,
    3.42 +// e.g.: "static ObjectMonitor * volatile gBlockList;"
    3.43 +#define CHECK_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY(typeName, fieldName, type)       \
    3.44 + {type volatile * dummy = &typeName::fieldName; }
    3.45 +
    3.46  // This macro ensures the type of a field and its containing type are
    3.47  // present in the type table. The assertion string is shorter than
    3.48  // preferable because (incredibly) of a bug in Solstice NFS client
    3.49 @@ -2889,6 +2900,7 @@
    3.50  
    3.51    VM_STRUCTS(GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
    3.52               GENERATE_STATIC_VM_STRUCT_ENTRY,
    3.53 +             GENERATE_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY,
    3.54               GENERATE_UNCHECKED_NONSTATIC_VM_STRUCT_ENTRY,
    3.55               GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
    3.56               GENERATE_NONPRODUCT_NONSTATIC_VM_STRUCT_ENTRY,
    3.57 @@ -3047,6 +3059,7 @@
    3.58  VMStructs::init() {
    3.59    VM_STRUCTS(CHECK_NONSTATIC_VM_STRUCT_ENTRY,
    3.60               CHECK_STATIC_VM_STRUCT_ENTRY,
    3.61 +             CHECK_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY,
    3.62               CHECK_NO_OP,
    3.63               CHECK_VOLATILE_NONSTATIC_VM_STRUCT_ENTRY,
    3.64               CHECK_NONPRODUCT_NONSTATIC_VM_STRUCT_ENTRY,
    3.65 @@ -3162,9 +3175,11 @@
    3.66                          CHECK_NO_OP,
    3.67                          CHECK_NO_OP,
    3.68                          CHECK_NO_OP,
    3.69 +                        CHECK_NO_OP,
    3.70                          CHECK_NO_OP));
    3.71    debug_only(VM_STRUCTS(CHECK_NO_OP,
    3.72                          ENSURE_FIELD_TYPE_PRESENT,
    3.73 +                        ENSURE_FIELD_TYPE_PRESENT,
    3.74                          CHECK_NO_OP,
    3.75                          ENSURE_FIELD_TYPE_PRESENT,
    3.76                          ENSURE_NONPRODUCT_FIELD_TYPE_PRESENT,

mercurial