Skip to content

Commit

Permalink
8230003: Make Monitor inherit from Mutex
Browse files Browse the repository at this point in the history
Reverse inheritance that makes more sense.

Reviewed-by: dholmes, rehn, pchilanomate
  • Loading branch information
coleenp committed Aug 22, 2019
1 parent a405118 commit da18495
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 166 deletions.
4 changes: 2 additions & 2 deletions src/hotspot/share/compiler/compileBroker.cpp
Expand Up @@ -2674,8 +2674,8 @@ void CompileBroker::print_heapinfo(outputStream* out, const char* function, size
// for the entire duration of aggregation and printing. That makes sure
// we see a consistent picture and do not run into issues caused by
// the CodeHeap being altered concurrently.
Monitor* global_lock = allFun ? CodeCache_lock : NULL;
Monitor* function_lock = allFun ? NULL : CodeCache_lock;
Mutex* global_lock = allFun ? CodeCache_lock : NULL;
Mutex* function_lock = allFun ? NULL : CodeCache_lock;
ts_global.update(); // record starting point
MutexLocker mu2(global_lock, Mutex::_no_safepoint_check_flag);
if (global_lock != NULL) {
Expand Down
Expand Up @@ -356,9 +356,9 @@ static void prepare_for_emergency_dump(Thread* thread) {
}

#ifdef ASSERT
Monitor* owned_lock = thread->owned_locks();
Mutex* owned_lock = thread->owned_locks();
while (owned_lock != NULL) {
Monitor* next = owned_lock->next();
Mutex* next = owned_lock->next();
owned_lock->unlock();
owned_lock = next;
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/logging/logTag.hpp
Expand Up @@ -171,7 +171,7 @@
LOG_TAG(mirror) \
LOG_TAG(verification) \
LOG_TAG(verify) \
LOG_TAG(vmmonitor) \
LOG_TAG(vmmutex) \
LOG_TAG(vmoperation) \
LOG_TAG(vmthread) \
LOG_TAG(vtables) \
Expand Down
30 changes: 15 additions & 15 deletions src/hotspot/share/runtime/interfaceSupport.inline.hpp
Expand Up @@ -254,30 +254,30 @@ class ThreadBlockInVM : public ThreadStateTransition {
};

// Unlike ThreadBlockInVM, this class is designed to avoid certain deadlock scenarios while making
// transitions inside class Monitor in cases where we need to block for a safepoint or handshake. It
// receives an extra argument compared to ThreadBlockInVM, the address of a pointer to the monitor we
// are trying to acquire. This will be used to access and release the monitor if needed to avoid
// transitions inside class Mutex in cases where we need to block for a safepoint or handshake. It
// receives an extra argument compared to ThreadBlockInVM, the address of a pointer to the mutex we
// are trying to acquire. This will be used to access and release the mutex if needed to avoid
// said deadlocks.
// It works like ThreadBlockInVM but differs from it in two ways:
// - When transitioning in (constructor), it checks for safepoints without blocking, i.e., calls
// back if needed to allow a pending safepoint to continue but does not block in it.
// - When transitioning back (destructor), if there is a pending safepoint or handshake it releases
// the monitor that is only partially acquired.
// the mutex that is only partially acquired.
class ThreadBlockInVMWithDeadlockCheck : public ThreadStateTransition {
private:
Monitor** _in_flight_monitor_adr;

void release_monitor() {
assert(_in_flight_monitor_adr != NULL, "_in_flight_monitor_adr should have been set on constructor");
Monitor* in_flight_monitor = *_in_flight_monitor_adr;
if (in_flight_monitor != NULL) {
in_flight_monitor->release_for_safepoint();
*_in_flight_monitor_adr = NULL;
Mutex** _in_flight_mutex_addr;

void release_mutex() {
assert(_in_flight_mutex_addr != NULL, "_in_flight_mutex_addr should have been set on constructor");
Mutex* in_flight_mutex = *_in_flight_mutex_addr;
if (in_flight_mutex != NULL) {
in_flight_mutex->release_for_safepoint();
*_in_flight_mutex_addr = NULL;
}
}
public:
ThreadBlockInVMWithDeadlockCheck(JavaThread* thread, Monitor** in_flight_monitor_adr)
: ThreadStateTransition(thread), _in_flight_monitor_adr(in_flight_monitor_adr) {
ThreadBlockInVMWithDeadlockCheck(JavaThread* thread, Mutex** in_flight_mutex_addr)
: ThreadStateTransition(thread), _in_flight_mutex_addr(in_flight_mutex_addr) {
// Once we are blocked vm expects stack to be walkable
thread->frame_anchor()->make_walkable(thread);

Expand All @@ -293,7 +293,7 @@ class ThreadBlockInVMWithDeadlockCheck : public ThreadStateTransition {
_thread->set_thread_state_fence((JavaThreadState)(_thread_blocked_trans));

if (SafepointMechanism::should_block(_thread)) {
release_monitor();
release_mutex();
SafepointMechanism::block_if_requested(_thread);
}

Expand Down
90 changes: 45 additions & 45 deletions src/hotspot/share/runtime/mutex.cpp
Expand Up @@ -33,27 +33,27 @@
#include "utilities/macros.hpp"

#ifdef ASSERT
void Monitor::check_safepoint_state(Thread* thread, bool do_safepoint_check) {
void Mutex::check_safepoint_state(Thread* thread, bool do_safepoint_check) {
// If the JavaThread checks for safepoint, verify that the lock wasn't created with safepoint_check_never.
SafepointCheckRequired not_allowed = do_safepoint_check ? Monitor::_safepoint_check_never :
Monitor::_safepoint_check_always;
SafepointCheckRequired not_allowed = do_safepoint_check ? Mutex::_safepoint_check_never :
Mutex::_safepoint_check_always;
assert(!thread->is_active_Java_thread() || _safepoint_check_required != not_allowed,
"This lock should %s have a safepoint check for Java threads: %s",
_safepoint_check_required ? "always" : "never", name());

// If defined with safepoint_check_never, a NonJavaThread should never ask to safepoint check either.
assert(thread->is_Java_thread() || !do_safepoint_check || _safepoint_check_required != Monitor::_safepoint_check_never,
assert(thread->is_Java_thread() || !do_safepoint_check || _safepoint_check_required != Mutex::_safepoint_check_never,
"NonJavaThread should not check for safepoint");
}
#endif // ASSERT

void Monitor::lock(Thread * self) {
void Mutex::lock(Thread * self) {
check_safepoint_state(self, true);

DEBUG_ONLY(check_prelock_state(self, true));
assert(_owner != self, "invariant");

Monitor* in_flight_monitor = NULL;
Mutex* in_flight_mutex = NULL;
DEBUG_ONLY(int retry_cnt = 0;)
bool is_active_Java_thread = self->is_active_Java_thread();
while (!_lock.try_lock()) {
Expand All @@ -62,18 +62,18 @@ void Monitor::lock(Thread * self) {
#ifdef ASSERT
check_block_state(self);
if (retry_cnt++ > 3) {
log_trace(vmmonitor)("JavaThread " INTPTR_FORMAT " on %d attempt trying to acquire vmmonitor %s", p2i(self), retry_cnt, _name);
log_trace(vmmutex)("JavaThread " INTPTR_FORMAT " on %d attempt trying to acquire vmmutex %s", p2i(self), retry_cnt, _name);
}
#endif // ASSERT

// Is it a JavaThread participating in the safepoint protocol.
if (is_active_Java_thread) {
assert(rank() > Mutex::special, "Potential deadlock with special or lesser rank mutex");
{ ThreadBlockInVMWithDeadlockCheck tbivmdc((JavaThread *) self, &in_flight_monitor);
in_flight_monitor = this; // save for ~ThreadBlockInVMWithDeadlockCheck
{ ThreadBlockInVMWithDeadlockCheck tbivmdc((JavaThread *) self, &in_flight_mutex);
in_flight_mutex = this; // save for ~ThreadBlockInVMWithDeadlockCheck
_lock.lock();
}
if (in_flight_monitor != NULL) {
if (in_flight_mutex != NULL) {
// Not unlocked by ~ThreadBlockInVMWithDeadlockCheck
break;
}
Expand All @@ -87,7 +87,7 @@ void Monitor::lock(Thread * self) {
set_owner(self);
}

void Monitor::lock() {
void Mutex::lock() {
this->lock(Thread::current());
}

Expand All @@ -97,22 +97,22 @@ void Monitor::lock() {
// safepoint-safe and so will prevent a safepoint from being reached. If used
// in the wrong way this can lead to a deadlock with the safepoint code.

void Monitor::lock_without_safepoint_check(Thread * self) {
void Mutex::lock_without_safepoint_check(Thread * self) {
check_safepoint_state(self, false);
assert(_owner != self, "invariant");
_lock.lock();
assert_owner(NULL);
set_owner(self);
}

void Monitor::lock_without_safepoint_check() {
void Mutex::lock_without_safepoint_check() {
lock_without_safepoint_check(Thread::current());
}


// Returns true if thread succeeds in grabbing the lock, otherwise false.

bool Monitor::try_lock() {
bool Mutex::try_lock() {
Thread * const self = Thread::current();
DEBUG_ONLY(check_prelock_state(self, false);)

Expand All @@ -124,12 +124,12 @@ bool Monitor::try_lock() {
return false;
}

void Monitor::release_for_safepoint() {
void Mutex::release_for_safepoint() {
assert_owner(NULL);
_lock.unlock();
}

void Monitor::unlock() {
void Mutex::unlock() {
assert_owner(Thread::current());
set_owner(NULL);
_lock.unlock();
Expand All @@ -147,7 +147,7 @@ void Monitor::notify_all() {

#ifdef ASSERT
void Monitor::assert_wait_lock_state(Thread* self) {
Monitor* least = get_least_ranked_lock_besides_this(self->owned_locks());
Mutex* least = get_least_ranked_lock_besides_this(self->owned_locks());
assert(least != this, "Specification of get_least_... call above");
if (least != NULL && least->rank() <= special) {
::tty->print("Attempting to wait on monitor %s/%d while holding"
Expand Down Expand Up @@ -194,10 +194,10 @@ bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
// abdicating the lock in wait
set_owner(NULL);
JavaThread *jt = (JavaThread *)self;
Monitor* in_flight_monitor = NULL;
Mutex* in_flight_mutex = NULL;

{
ThreadBlockInVMWithDeadlockCheck tbivmdc(jt, &in_flight_monitor);
ThreadBlockInVMWithDeadlockCheck tbivmdc(jt, &in_flight_mutex);
OSThreadWaitState osts(self->osthread(), false /* not Object.wait() */);
if (as_suspend_equivalent) {
jt->set_suspend_equivalent();
Expand All @@ -206,7 +206,7 @@ bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
}

wait_status = _lock.wait(timeout);
in_flight_monitor = this; // save for ~ThreadBlockInVMWithDeadlockCheck
in_flight_mutex = this; // save for ~ThreadBlockInVMWithDeadlockCheck

// were we externally suspended while we were waiting?
if (as_suspend_equivalent && jt->handle_special_suspend_equivalent_condition()) {
Expand All @@ -220,7 +220,7 @@ bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
}
}

if (in_flight_monitor != NULL) {
if (in_flight_mutex != NULL) {
// Not unlocked by ~ThreadBlockInVMWithDeadlockCheck
assert_owner(NULL);
// Conceptually reestablish ownership of the lock.
Expand All @@ -232,7 +232,7 @@ bool Monitor::wait(long timeout, bool as_suspend_equivalent) {
return wait_status != 0; // return true IFF timeout
}

Monitor::~Monitor() {
Mutex::~Mutex() {
assert_owner(NULL);
}

Expand All @@ -241,34 +241,34 @@ bool is_sometimes_ok(const char* name) {
return (strcmp(name, "Threads_lock") == 0 || strcmp(name, "Heap_lock") == 0 || strcmp(name, "SR_lock") == 0);
}

Monitor::Monitor(int Rank, const char * name, bool allow_vm_block,
SafepointCheckRequired safepoint_check_required) : _owner(NULL) {
Mutex::Mutex(int Rank, const char * name, bool allow_vm_block,
SafepointCheckRequired safepoint_check_required) : _owner(NULL) {
assert(os::mutex_init_done(), "Too early!");
if (name == NULL) {
strcpy(_name, "UNKNOWN");
} else {
strncpy(_name, name, MONITOR_NAME_LEN - 1);
_name[MONITOR_NAME_LEN - 1] = '\0';
strncpy(_name, name, MUTEX_NAME_LEN - 1);
_name[MUTEX_NAME_LEN - 1] = '\0';
}
#ifdef ASSERT
_allow_vm_block = allow_vm_block;
_rank = Rank;
_safepoint_check_required = safepoint_check_required;

assert(_safepoint_check_required != Monitor::_safepoint_check_sometimes || is_sometimes_ok(name),
assert(_safepoint_check_required != Mutex::_safepoint_check_sometimes || is_sometimes_ok(name),
"Lock has _safepoint_check_sometimes %s", name);
#endif
}

Mutex::Mutex(int Rank, const char * name, bool allow_vm_block,
Monitor::Monitor(int Rank, const char * name, bool allow_vm_block,
SafepointCheckRequired safepoint_check_required) :
Monitor(Rank, name, allow_vm_block, safepoint_check_required) {}
Mutex(Rank, name, allow_vm_block, safepoint_check_required) {}

bool Monitor::owned_by_self() const {
bool Mutex::owned_by_self() const {
return _owner == Thread::current();
}

void Monitor::print_on_error(outputStream* st) const {
void Mutex::print_on_error(outputStream* st) const {
st->print("[" PTR_FORMAT, p2i(this));
st->print("] %s", _name);
st->print(" - owner thread: " PTR_FORMAT, p2i(_owner));
Expand All @@ -278,7 +278,7 @@ void Monitor::print_on_error(outputStream* st) const {
// Non-product code

#ifndef PRODUCT
void Monitor::print_on(outputStream* st) const {
void Mutex::print_on(outputStream* st) const {
st->print_cr("Mutex: [" PTR_FORMAT "] %s - owner: " PTR_FORMAT,
p2i(this), _name, p2i(_owner));
}
Expand All @@ -287,7 +287,7 @@ void Monitor::print_on(outputStream* st) const {
#ifndef PRODUCT
#ifdef ASSERT

void Monitor::assert_owner(Thread * expected) {
void Mutex::assert_owner(Thread * expected) {
const char* msg = "invalid owner";
if (expected == NULL) {
msg = "should be un-owned";
Expand All @@ -300,8 +300,8 @@ void Monitor::assert_owner(Thread * expected) {
msg, p2i(_owner), p2i(expected));
}

Monitor * Monitor::get_least_ranked_lock(Monitor * locks) {
Monitor *res, *tmp;
Mutex* Mutex::get_least_ranked_lock(Mutex* locks) {
Mutex *res, *tmp;
for (res = tmp = locks; tmp != NULL; tmp = tmp->next()) {
if (tmp->rank() < res->rank()) {
res = tmp;
Expand All @@ -320,8 +320,8 @@ Monitor * Monitor::get_least_ranked_lock(Monitor * locks) {
return res;
}

Monitor* Monitor::get_least_ranked_lock_besides_this(Monitor* locks) {
Monitor *res, *tmp;
Mutex* Mutex::get_least_ranked_lock_besides_this(Mutex* locks) {
Mutex *res, *tmp;
for (res = NULL, tmp = locks; tmp != NULL; tmp = tmp->next()) {
if (tmp != this && (res == NULL || tmp->rank() < res->rank())) {
res = tmp;
Expand All @@ -341,7 +341,7 @@ Monitor* Monitor::get_least_ranked_lock_besides_this(Monitor* locks) {
}


bool Monitor::contains(Monitor* locks, Monitor * lock) {
bool Mutex::contains(Mutex* locks, Mutex* lock) {
for (; locks != NULL; locks = locks->next()) {
if (locks == lock) {
return true;
Expand All @@ -356,7 +356,7 @@ bool Monitor::contains(Monitor* locks, Monitor * lock) {
// might indicate exposure to deadlock.
// Rather like an EventListener for _owner (:>).

void Monitor::set_owner_implementation(Thread *new_owner) {
void Mutex::set_owner_implementation(Thread *new_owner) {
// This function is solely responsible for maintaining
// and checking the invariant that threads and locks
// are in a 1/N relation, with some some locks unowned.
Expand All @@ -377,7 +377,7 @@ void Monitor::set_owner_implementation(Thread *new_owner) {
// link "this" into the owned locks list

#ifdef ASSERT // Thread::_owned_locks is under the same ifdef
Monitor* locks = get_least_ranked_lock(new_owner->owned_locks());
Mutex* locks = get_least_ranked_lock(new_owner->owned_locks());
// Mutex::set_owner_implementation is a friend of Thread

assert(this->rank() >= 0, "bad lock rank");
Expand Down Expand Up @@ -415,11 +415,11 @@ void Monitor::set_owner_implementation(Thread *new_owner) {
_owner = NULL; // set the owner

#ifdef ASSERT
Monitor *locks = old_owner->owned_locks();
Mutex* locks = old_owner->owned_locks();

// remove "this" from the owned locks list

Monitor *prev = NULL;
Mutex* prev = NULL;
bool found = false;
for (; locks != NULL; prev = locks, locks = locks->next()) {
if (locks == this) {
Expand All @@ -440,7 +440,7 @@ void Monitor::set_owner_implementation(Thread *new_owner) {


// Factored out common sanity checks for locking mutex'es. Used by lock() and try_lock()
void Monitor::check_prelock_state(Thread *thread, bool safepoint_check) {
void Mutex::check_prelock_state(Thread *thread, bool safepoint_check) {
if (safepoint_check) {
assert((!thread->is_active_Java_thread() || ((JavaThread *)thread)->thread_state() == _thread_in_vm)
|| rank() == Mutex::special, "wrong thread state for using locks");
Expand All @@ -454,13 +454,13 @@ void Monitor::check_prelock_state(Thread *thread, bool safepoint_check) {
"locking not allowed when crash protection is set");
}

void Monitor::check_block_state(Thread *thread) {
void Mutex::check_block_state(Thread *thread) {
if (!_allow_vm_block && thread->is_VM_thread()) {
warning("VM thread blocked on lock");
print();
BREAKPOINT;
}
assert(_owner != thread, "deadlock: blocking on monitor owned by current thread");
assert(_owner != thread, "deadlock: blocking on mutex owned by current thread");
}

#endif // PRODUCT

0 comments on commit da18495

Please sign in to comment.