269 lines
12 KiB
Diff
269 lines
12 KiB
Diff
From 752e6b3d3747194e9ebc2a57c06403a3ed6fccc0 Mon Sep 17 00:00:00 2001
|
|
Date: Sat, 24 Aug 2019 17:39:22 +0000
|
|
Subject: [PATCH] Backport of JDK-8047212
|
|
|
|
summary: Fix race between ObjectMonitor alloc and verification code; teach SA about "static pointer volatile" fields
|
|
LLT:
|
|
|
|
Bug url: https://bugs.openjdk.java.net/browse/JDK-8047212
|
|
---
|
|
hotspot/src/share/vm/runtime/synchronizer.cpp | 94 ++++++++++++++-------------
|
|
hotspot/src/share/vm/runtime/synchronizer.hpp | 2 +-
|
|
hotspot/src/share/vm/runtime/vmStructs.cpp | 19 +++++-
|
|
3 files changed, 68 insertions(+), 47 deletions(-)
|
|
|
|
diff --git a/hotspot/src/share/vm/runtime/synchronizer.cpp b/hotspot/src/share/vm/runtime/synchronizer.cpp
|
|
index 53341523c8..6e9c1da6bb 100644
|
|
--- a/hotspot/src/share/vm/runtime/synchronizer.cpp
|
|
+++ b/hotspot/src/share/vm/runtime/synchronizer.cpp
|
|
@@ -149,7 +149,7 @@ int dtrace_waited_probe(ObjectMonitor* monitor, Handle obj, Thread* thr) {
|
|
#define NINFLATIONLOCKS 256
|
|
static volatile intptr_t InflationLocks [NINFLATIONLOCKS] ;
|
|
|
|
-ObjectMonitor * ObjectSynchronizer::gBlockList = NULL ;
|
|
+ObjectMonitor * volatile ObjectSynchronizer::gBlockList = NULL;
|
|
ObjectMonitor * volatile ObjectSynchronizer::gFreeList = NULL ;
|
|
ObjectMonitor * volatile ObjectSynchronizer::gOmInUseList = NULL ;
|
|
int ObjectSynchronizer::gOmInUseCount = 0;
|
|
@@ -830,18 +830,18 @@ JavaThread* ObjectSynchronizer::get_lock_owner(Handle h_obj, bool doLock) {
|
|
// Visitors ...
|
|
|
|
void ObjectSynchronizer::monitors_iterate(MonitorClosure* closure) {
|
|
- ObjectMonitor* block = gBlockList;
|
|
- ObjectMonitor* mid;
|
|
- while (block) {
|
|
+ ObjectMonitor* block =
|
|
+ (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
|
|
+ while (block != NULL) {
|
|
assert(block->object() == CHAINMARKER, "must be a block header");
|
|
for (int i = _BLOCKSIZE - 1; i > 0; i--) {
|
|
- mid = block + i;
|
|
- oop object = (oop) mid->object();
|
|
+ ObjectMonitor* mid = (ObjectMonitor *)(block + i);
|
|
+ oop object = (oop)mid->object();
|
|
if (object != NULL) {
|
|
closure->do_monitor(mid);
|
|
}
|
|
}
|
|
- block = (ObjectMonitor*) block->FreeNext;
|
|
+ block = (ObjectMonitor*)block->FreeNext;
|
|
}
|
|
}
|
|
|
|
@@ -867,7 +867,9 @@ void ObjectSynchronizer::oops_do(OopClosure* f) {
|
|
|
|
void ObjectSynchronizer::oops_do(OopClosure* f) {
|
|
assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
|
|
- for (ObjectMonitor* block = gBlockList; block != NULL; block = next(block)) {
|
|
+ ObjectMonitor* block =
|
|
+ (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
|
|
+ for (; block != NULL; block = (ObjectMonitor *)next(block)) {
|
|
assert(block->object() == CHAINMARKER, "must be a block header");
|
|
for (int i = 1; i < _BLOCKSIZE; i++) {
|
|
ObjectMonitor* mid = &block[i];
|
|
@@ -1090,7 +1092,9 @@ ObjectMonitor * ATTR ObjectSynchronizer::omAlloc (Thread * Self) {
|
|
// The very first objectMonitor in a block is reserved and dedicated.
|
|
// It serves as blocklist "next" linkage.
|
|
temp[0].FreeNext = gBlockList;
|
|
- gBlockList = temp;
|
|
+ // There are lock-free uses of gBlockList so make sure that
|
|
+ // the previous stores happen before we update gBlockList.
|
|
+ OrderAccess::release_store_ptr(&gBlockList, temp);
|
|
|
|
// Add the new string of objectMonitors to the global free list
|
|
temp[_BLOCKSIZE - 1].FreeNext = gFreeList ;
|
|
@@ -1569,29 +1573,31 @@ void ObjectSynchronizer::deflate_idle_monitors() {
|
|
nInuse += gOmInUseCount;
|
|
}
|
|
|
|
- } else for (ObjectMonitor* block = gBlockList; block != NULL; block = next(block)) {
|
|
- // Iterate over all extant monitors - Scavenge all idle monitors.
|
|
- assert(block->object() == CHAINMARKER, "must be a block header");
|
|
- nInCirculation += _BLOCKSIZE ;
|
|
- for (int i = 1 ; i < _BLOCKSIZE; i++) {
|
|
- ObjectMonitor* mid = &block[i];
|
|
- oop obj = (oop) mid->object();
|
|
-
|
|
- if (obj == NULL) {
|
|
- // The monitor is not associated with an object.
|
|
- // The monitor should either be a thread-specific private
|
|
- // free list or the global free list.
|
|
- // obj == NULL IMPLIES mid->is_busy() == 0
|
|
- guarantee (!mid->is_busy(), "invariant") ;
|
|
- continue ;
|
|
- }
|
|
- deflated = deflate_monitor(mid, obj, &FreeHead, &FreeTail);
|
|
-
|
|
- if (deflated) {
|
|
- mid->FreeNext = NULL ;
|
|
- nScavenged ++ ;
|
|
- } else {
|
|
- nInuse ++;
|
|
+ } else {
|
|
+ ObjectMonitor* block =
|
|
+ (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
|
|
+ for (; block != NULL; block = (ObjectMonitor*)next(block)) {
|
|
+ // Iterate over all extant monitors - Scavenge all idle monitors.
|
|
+ assert(block->object() == CHAINMARKER, "must be a block header");
|
|
+ nInCirculation += _BLOCKSIZE;
|
|
+ for (int i = 1; i < _BLOCKSIZE; i++) {
|
|
+ ObjectMonitor* mid = (ObjectMonitor*)&block[i];
|
|
+ oop obj = (oop)mid->object();
|
|
+ if (obj == NULL) {
|
|
+ // The monitor is not associated with an object.
|
|
+ // The monitor should either be a thread-specific private
|
|
+ // free list or the global free list.
|
|
+ // obj == NULL IMPLIES mid->is_busy() == 0
|
|
+ guarantee(!mid->is_busy(), "invariant");
|
|
+ continue;
|
|
+ }
|
|
+ deflated = deflate_monitor(mid, obj, &FreeHead, &FreeTail);
|
|
+ if (deflated) {
|
|
+ mid->FreeNext = NULL;
|
|
+ nScavenged++;
|
|
+ } else {
|
|
+ nInuse++;
|
|
+ }
|
|
}
|
|
}
|
|
}
|
|
@@ -1726,13 +1732,13 @@ void ObjectSynchronizer::sanity_checks(const bool verbose,
|
|
|
|
// Verify all monitors in the monitor cache, the verification is weak.
|
|
void ObjectSynchronizer::verify() {
|
|
- ObjectMonitor* block = gBlockList;
|
|
- ObjectMonitor* mid;
|
|
- while (block) {
|
|
+ ObjectMonitor* block =
|
|
+ (ObjectMonitor *)OrderAccess::load_ptr_acquire(&gBlockList);
|
|
+ while (block != NULL) {
|
|
assert(block->object() == CHAINMARKER, "must be a block header");
|
|
for (int i = 1; i < _BLOCKSIZE; i++) {
|
|
- mid = block + i;
|
|
- oop object = (oop) mid->object();
|
|
+ ObjectMonitor* mid = (ObjectMonitor *)(block + i);
|
|
+ oop object = (oop)mid->object();
|
|
if (object != NULL) {
|
|
mid->verify();
|
|
}
|
|
@@ -1746,18 +1752,18 @@ void ObjectSynchronizer::verify() {
|
|
// the list of extant blocks without taking a lock.
|
|
|
|
int ObjectSynchronizer::verify_objmon_isinpool(ObjectMonitor *monitor) {
|
|
- ObjectMonitor* block = gBlockList;
|
|
-
|
|
- while (block) {
|
|
+ ObjectMonitor* block =
|
|
+ (ObjectMonitor*)OrderAccess::load_ptr_acquire(&gBlockList);
|
|
+ while (block != NULL) {
|
|
assert(block->object() == CHAINMARKER, "must be a block header");
|
|
if (monitor > &block[0] && monitor < &block[_BLOCKSIZE]) {
|
|
- address mon = (address) monitor;
|
|
- address blk = (address) block;
|
|
+ address mon = (address)monitor;
|
|
+ address blk = (address)block;
|
|
size_t diff = mon - blk;
|
|
- assert((diff % sizeof(ObjectMonitor)) == 0, "check");
|
|
+ assert((diff % sizeof(ObjectMonitor)) == 0, "must be aligned");
|
|
return 1;
|
|
}
|
|
- block = (ObjectMonitor*) block->FreeNext;
|
|
+ block = (ObjectMonitor*)block->FreeNext;
|
|
}
|
|
return 0;
|
|
}
|
|
diff --git a/hotspot/src/share/vm/runtime/synchronizer.hpp b/hotspot/src/share/vm/runtime/synchronizer.hpp
|
|
index 3247776b74..4c4a7155af 100644
|
|
--- a/hotspot/src/share/vm/runtime/synchronizer.hpp
|
|
+++ b/hotspot/src/share/vm/runtime/synchronizer.hpp
|
|
@@ -151,7 +151,7 @@ class ObjectSynchronizer : AllStatic {
|
|
|
|
private:
|
|
enum { _BLOCKSIZE = 128 };
|
|
- static ObjectMonitor* gBlockList;
|
|
+ static ObjectMonitor * volatile gBlockList;
|
|
static ObjectMonitor * volatile gFreeList;
|
|
static ObjectMonitor * volatile gOmInUseList; // for moribund thread, so monitors they inflated still get scanned
|
|
static int gOmInUseCount;
|
|
diff --git a/hotspot/src/share/vm/runtime/vmStructs.cpp b/hotspot/src/share/vm/runtime/vmStructs.cpp
|
|
index 5b5282e38e..7935f8cb47 100644
|
|
--- a/hotspot/src/share/vm/runtime/vmStructs.cpp
|
|
+++ b/hotspot/src/share/vm/runtime/vmStructs.cpp
|
|
@@ -267,6 +267,7 @@ typedef TwoOopHashtable<Symbol*, mtClass> SymbolTwoOopHashtable;
|
|
|
|
#define VM_STRUCTS(nonstatic_field, \
|
|
static_field, \
|
|
+ static_ptr_volatile_field, \
|
|
unchecked_nonstatic_field, \
|
|
volatile_nonstatic_field, \
|
|
nonproduct_nonstatic_field, \
|
|
@@ -1091,7 +1092,7 @@ typedef TwoOopHashtable<Symbol*, mtClass> SymbolTwoOopHashtable;
|
|
volatile_nonstatic_field(BasicLock, _displaced_header, markOop) \
|
|
nonstatic_field(BasicObjectLock, _lock, BasicLock) \
|
|
nonstatic_field(BasicObjectLock, _obj, oop) \
|
|
- static_field(ObjectSynchronizer, gBlockList, ObjectMonitor*) \
|
|
+ static_ptr_volatile_field(ObjectSynchronizer,gBlockList, ObjectMonitor*) \
|
|
\
|
|
/*********************/ \
|
|
/* Matcher (C2 only) */ \
|
|
@@ -2682,6 +2683,11 @@ typedef TwoOopHashtable<Symbol*, mtClass> SymbolTwoOopHashtable;
|
|
#define GENERATE_STATIC_VM_STRUCT_ENTRY(typeName, fieldName, type) \
|
|
{ QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, &typeName::fieldName },
|
|
|
|
+// This macro generates a VMStructEntry line for a static pointer volatile field,
|
|
+// e.g.: "static ObjectMonitor * volatile gBlockList;"
|
|
+#define GENERATE_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY(typeName, fieldName, type) \
|
|
+ { QUOTE(typeName), QUOTE(fieldName), QUOTE(type), 1, 0, (void*)&typeName::fieldName },
|
|
+
|
|
// This macro generates a VMStructEntry line for an unchecked
|
|
// nonstatic field, in which the size of the type is also specified.
|
|
// The type string is given as NULL, indicating an "opaque" type.
|
|
@@ -2707,10 +2713,15 @@ typedef TwoOopHashtable<Symbol*, mtClass> SymbolTwoOopHashtable;
|
|
#define CHECK_VOLATILE_NONSTATIC_VM_STRUCT_ENTRY(typeName, fieldName, type) \
|
|
{typedef type dummyvtype; typeName *dummyObj = NULL; volatile dummyvtype* dummy = &dummyObj->fieldName; }
|
|
|
|
-// This macro checks the type of a VMStructEntry by comparing pointer types
|
|
+// This macro checks the type of a static VMStructEntry by comparing pointer types
|
|
#define CHECK_STATIC_VM_STRUCT_ENTRY(typeName, fieldName, type) \
|
|
{type* dummy = &typeName::fieldName; }
|
|
|
|
+// This macro checks the type of a static pointer volatile VMStructEntry by comparing pointer types,
|
|
+// e.g.: "static ObjectMonitor * volatile gBlockList;"
|
|
+#define CHECK_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY(typeName, fieldName, type) \
|
|
+ {type volatile * dummy = &typeName::fieldName; }
|
|
+
|
|
// This macro ensures the type of a field and its containing type are
|
|
// present in the type table. The assertion string is shorter than
|
|
// preferable because (incredibly) of a bug in Solstice NFS client
|
|
@@ -2904,6 +2915,7 @@ VMStructEntry VMStructs::localHotSpotVMStructs[] = {
|
|
|
|
VM_STRUCTS(GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
|
|
GENERATE_STATIC_VM_STRUCT_ENTRY,
|
|
+ GENERATE_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY,
|
|
GENERATE_UNCHECKED_NONSTATIC_VM_STRUCT_ENTRY,
|
|
GENERATE_NONSTATIC_VM_STRUCT_ENTRY,
|
|
GENERATE_NONPRODUCT_NONSTATIC_VM_STRUCT_ENTRY,
|
|
@@ -3074,6 +3086,7 @@ void
|
|
VMStructs::init() {
|
|
VM_STRUCTS(CHECK_NONSTATIC_VM_STRUCT_ENTRY,
|
|
CHECK_STATIC_VM_STRUCT_ENTRY,
|
|
+ CHECK_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY,
|
|
CHECK_NO_OP,
|
|
CHECK_VOLATILE_NONSTATIC_VM_STRUCT_ENTRY,
|
|
CHECK_NONPRODUCT_NONSTATIC_VM_STRUCT_ENTRY,
|
|
@@ -3196,9 +3209,11 @@ VMStructs::init() {
|
|
CHECK_NO_OP,
|
|
CHECK_NO_OP,
|
|
CHECK_NO_OP,
|
|
+ CHECK_NO_OP,
|
|
CHECK_NO_OP));
|
|
debug_only(VM_STRUCTS(CHECK_NO_OP,
|
|
ENSURE_FIELD_TYPE_PRESENT,
|
|
+ ENSURE_FIELD_TYPE_PRESENT,
|
|
CHECK_NO_OP,
|
|
ENSURE_FIELD_TYPE_PRESENT,
|
|
ENSURE_NONPRODUCT_FIELD_TYPE_PRESENT,
|
|
--
|
|
2.12.3
|
|
|