openjdk-1.8.0/8047212.patch

270 lines
12 KiB
Diff

From 752e6b3d3747194e9ebc2a57c06403a3ed6fccc0 Mon Sep 17 00:00:00 2001
From: guoxuanda <guoxuanda@huawei.com>
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