Skip to content

Commit aabc9ca

Browse files
committed
8255678: Add Mutex::try_lock version without rank checks
Reviewed-by: dcubed, dholmes, coleenp
1 parent 884b9ff commit aabc9ca

File tree

3 files changed

+273
-82
lines changed

3 files changed

+273
-82
lines changed

src/hotspot/share/runtime/mutex.cpp

Lines changed: 85 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,11 @@ void Mutex::lock_contended(Thread* self) {
100100
}
101101

102102
void Mutex::lock(Thread* self) {
103-
check_safepoint_state(self);
104-
105103
assert(_owner != self, "invariant");
106104

105+
check_safepoint_state(self);
106+
check_rank(self);
107+
107108
if (!_lock.try_lock()) {
108109
// The lock is contended, use contended slow-path function to lock
109110
lock_contended(self);
@@ -124,8 +125,11 @@ void Mutex::lock() {
124125
// in the wrong way this can lead to a deadlock with the safepoint code.
125126

126127
void Mutex::lock_without_safepoint_check(Thread * self) {
127-
check_no_safepoint_state(self);
128128
assert(_owner != self, "invariant");
129+
130+
check_no_safepoint_state(self);
131+
check_rank(self);
132+
129133
_lock.lock();
130134
assert_owner(NULL);
131135
set_owner(self);
@@ -137,21 +141,39 @@ void Mutex::lock_without_safepoint_check() {
137141

138142

139143
// Returns true if thread succeeds in grabbing the lock, otherwise false.
140-
bool Mutex::try_lock() {
144+
bool Mutex::try_lock_inner(bool do_rank_checks) {
141145
Thread * const self = Thread::current();
146+
// Checking the owner hides the potential difference in recursive locking behaviour
147+
// on some platforms.
148+
if (_owner == self) {
149+
return false;
150+
}
151+
152+
if (do_rank_checks) {
153+
check_rank(self);
154+
}
142155
// Some safepoint_check_always locks use try_lock, so cannot check
143156
// safepoint state, but can check blocking state.
144157
check_block_state(self);
145-
// Checking the owner hides the potential difference in recursive locking behaviour
146-
// on some platforms.
147-
if (_owner != self && _lock.try_lock()) {
158+
159+
if (_lock.try_lock()) {
148160
assert_owner(NULL);
149161
set_owner(self);
150162
return true;
151163
}
152164
return false;
153165
}
154166

167+
bool Mutex::try_lock() {
168+
return try_lock_inner(true /* do_rank_checks */);
169+
}
170+
171+
bool Mutex::try_lock_without_rank_check() {
172+
bool res = try_lock_inner(false /* do_rank_checks */);
173+
DEBUG_ONLY(if (res) _skip_rank_check = true;)
174+
return res;
175+
}
176+
155177
void Mutex::release_for_safepoint() {
156178
assert_owner(NULL);
157179
_lock.unlock();
@@ -173,31 +195,18 @@ void Monitor::notify_all() {
173195
_lock.notify_all();
174196
}
175197

176-
#ifdef ASSERT
177-
void Monitor::assert_wait_lock_state(Thread* self) {
178-
Mutex* least = get_least_ranked_lock_besides_this(self->owned_locks());
179-
assert(least != this, "Specification of get_least_... call above");
180-
if (least != NULL && least->rank() <= special) {
181-
::tty->print("Attempting to wait on monitor %s/%d while holding"
182-
" lock %s/%d -- possible deadlock",
183-
name(), rank(), least->name(), least->rank());
184-
assert(false, "Shouldn't block(wait) while holding a lock of rank special");
185-
}
186-
}
187-
#endif // ASSERT
188-
189198
bool Monitor::wait_without_safepoint_check(int64_t timeout) {
190199
Thread* const self = Thread::current();
191200

192201
// timeout is in milliseconds - with zero meaning never timeout
193202
assert(timeout >= 0, "negative timeout");
194-
195203
assert_owner(self);
196-
assert_wait_lock_state(self);
204+
check_rank(self);
197205

198206
// conceptually set the owner to NULL in anticipation of
199207
// abdicating the lock in wait
200208
set_owner(NULL);
209+
201210
// Check safepoint state after resetting owner and possible NSV.
202211
check_no_safepoint_state(self);
203212

@@ -208,23 +217,22 @@ bool Monitor::wait_without_safepoint_check(int64_t timeout) {
208217

209218
bool Monitor::wait(int64_t timeout, bool as_suspend_equivalent) {
210219
JavaThread* const self = JavaThread::current();
220+
// Safepoint checking logically implies an active JavaThread.
221+
assert(self->is_active_Java_thread(), "invariant");
211222

212223
// timeout is in milliseconds - with zero meaning never timeout
213224
assert(timeout >= 0, "negative timeout");
214-
215225
assert_owner(self);
226+
check_rank(self);
216227

217-
// Safepoint checking logically implies an active JavaThread.
218-
guarantee(self->is_active_Java_thread(), "invariant");
219-
assert_wait_lock_state(self);
220-
221-
int wait_status;
222228
// conceptually set the owner to NULL in anticipation of
223229
// abdicating the lock in wait
224230
set_owner(NULL);
231+
225232
// Check safepoint state after resetting owner and possible NSV.
226233
check_safepoint_state(self);
227234

235+
int wait_status;
228236
Mutex* in_flight_mutex = NULL;
229237

230238
{
@@ -285,6 +293,7 @@ Mutex::Mutex(int Rank, const char * name, bool allow_vm_block,
285293
_allow_vm_block = allow_vm_block;
286294
_rank = Rank;
287295
_safepoint_check_required = safepoint_check_required;
296+
_skip_rank_check = false;
288297

289298
assert(_safepoint_check_required != _safepoint_check_sometimes || is_sometimes_ok(name),
290299
"Lock has _safepoint_check_sometimes %s", name);
@@ -330,7 +339,7 @@ void Mutex::print_on(outputStream* st) const {
330339
st->print(" %s", print_safepoint_check(_safepoint_check_required));
331340
st->cr();
332341
}
333-
#endif
342+
#endif // PRODUCT
334343

335344
#ifdef ASSERT
336345
void Mutex::assert_owner(Thread * expected) {
@@ -353,16 +362,6 @@ Mutex* Mutex::get_least_ranked_lock(Mutex* locks) {
353362
res = tmp;
354363
}
355364
}
356-
if (!SafepointSynchronize::is_at_safepoint()) {
357-
// In this case, we expect the held locks to be
358-
// in increasing rank order (modulo any native ranks)
359-
for (tmp = locks; tmp != NULL; tmp = tmp->next()) {
360-
if (tmp->next() != NULL) {
361-
assert(tmp->rank() == Mutex::native ||
362-
tmp->rank() <= tmp->next()->rank(), "mutex rank anomaly?");
363-
}
364-
}
365-
}
366365
return res;
367366
}
368367

@@ -373,17 +372,56 @@ Mutex* Mutex::get_least_ranked_lock_besides_this(Mutex* locks) {
373372
res = tmp;
374373
}
375374
}
375+
assert(res != this, "invariant");
376+
return res;
377+
}
378+
379+
// Tests for rank violations that might indicate exposure to deadlock.
380+
void Mutex::check_rank(Thread* thread) {
381+
assert(this->rank() >= 0, "bad lock rank");
382+
Mutex* locks_owned = thread->owned_locks();
383+
376384
if (!SafepointSynchronize::is_at_safepoint()) {
377-
// In this case, we expect the held locks to be
378-
// in increasing rank order (modulo any native ranks)
379-
for (tmp = locks; tmp != NULL; tmp = tmp->next()) {
385+
// We expect the locks already acquired to be in increasing rank order,
386+
// modulo locks of native rank or acquired in try_lock_without_rank_check()
387+
for (Mutex* tmp = locks_owned; tmp != NULL; tmp = tmp->next()) {
380388
if (tmp->next() != NULL) {
381-
assert(tmp->rank() == Mutex::native ||
382-
tmp->rank() <= tmp->next()->rank(), "mutex rank anomaly?");
389+
assert(tmp->rank() == Mutex::native || tmp->rank() < tmp->next()->rank()
390+
|| tmp->skip_rank_check(), "mutex rank anomaly?");
383391
}
384392
}
385393
}
386-
return res;
394+
395+
// Locks with rank native or suspend_resume are an exception and are not
396+
// subject to the verification rules.
397+
bool check_can_be_skipped = this->rank() == Mutex::native || this->rank() == Mutex::suspend_resume
398+
|| SafepointSynchronize::is_at_safepoint();
399+
if (owned_by_self()) {
400+
// wait() case
401+
Mutex* least = get_least_ranked_lock_besides_this(locks_owned);
402+
// We enforce not holding locks of rank special or lower while waiting.
403+
// Also "this" should be the monitor with lowest rank owned by this thread.
404+
if (least != NULL && (least->rank() <= special ||
405+
(least->rank() <= this->rank() && !check_can_be_skipped))) {
406+
assert(false, "Attempting to wait on monitor %s/%d while holding lock %s/%d -- "
407+
"possible deadlock. %s", name(), rank(), least->name(), least->rank(),
408+
least->rank() <= this->rank() ? "Should wait on the least ranked monitor from "
409+
"all owned locks." : "Should not block(wait) while holding a lock of rank special.");
410+
}
411+
} else if (!check_can_be_skipped) {
412+
// lock()/lock_without_safepoint_check()/try_lock() case
413+
Mutex* least = get_least_ranked_lock(locks_owned);
414+
// Deadlock prevention rules require us to acquire Mutexes only in
415+
// a global total order. For example, if m1 is the lowest ranked mutex
416+
// that the thread holds and m2 is the mutex the thread is trying
417+
// to acquire, then deadlock prevention rules require that the rank
418+
// of m2 be less than the rank of m1. This prevents circular waits.
419+
if (least != NULL && least->rank() <= this->rank()) {
420+
thread->print_owned_locks();
421+
assert(false, "Attempting to acquire lock %s/%d out of order with lock %s/%d -- "
422+
"possible deadlock", this->name(), this->rank(), least->name(), least->rank());
423+
}
424+
}
387425
}
388426

389427
bool Mutex::contains(Mutex* locks, Mutex* lock) {
@@ -413,8 +451,7 @@ void Mutex::no_safepoint_verifier(Thread* thread, bool enable) {
413451
}
414452

415453
// Called immediately after lock acquisition or release as a diagnostic
416-
// to track the lock-set of the thread and test for rank violations that
417-
// might indicate exposure to deadlock.
454+
// to track the lock-set of the thread.
418455
// Rather like an EventListener for _owner (:>).
419456

420457
void Mutex::set_owner_implementation(Thread *new_owner) {
@@ -436,29 +473,6 @@ void Mutex::set_owner_implementation(Thread *new_owner) {
436473
_owner = new_owner; // set the owner
437474

438475
// link "this" into the owned locks list
439-
440-
Mutex* locks = get_least_ranked_lock(new_owner->owned_locks());
441-
// Mutex::set_owner_implementation is a friend of Thread
442-
443-
assert(this->rank() >= 0, "bad lock rank");
444-
445-
// Deadlock avoidance rules require us to acquire Mutexes only in
446-
// a global total order. For example m1 is the lowest ranked mutex
447-
// that the thread holds and m2 is the mutex the thread is trying
448-
// to acquire, then deadlock avoidance rules require that the rank
449-
// of m2 be less than the rank of m1.
450-
// The rank Mutex::native is an exception in that it is not subject
451-
// to the verification rules.
452-
if (this->rank() != Mutex::native &&
453-
this->rank() != Mutex::suspend_resume &&
454-
locks != NULL && locks->rank() <= this->rank() &&
455-
!SafepointSynchronize::is_at_safepoint()) {
456-
new_owner->print_owned_locks();
457-
fatal("acquiring lock %s/%d out of order with lock %s/%d -- "
458-
"possible deadlock", this->name(), this->rank(),
459-
locks->name(), locks->rank());
460-
}
461-
462476
this->_next = new_owner->_owned_locks;
463477
new_owner->_owned_locks = this;
464478

@@ -470,6 +484,7 @@ void Mutex::set_owner_implementation(Thread *new_owner) {
470484

471485
Thread* old_owner = _owner;
472486
_last_owner = old_owner;
487+
_skip_rank_check = false;
473488

474489
assert(old_owner != NULL, "removing the owner thread of an unowned mutex");
475490
assert(old_owner == Thread::current(), "removing the owner thread of an unowned mutex");

src/hotspot/share/runtime/mutex.hpp

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -89,18 +89,33 @@ class Mutex : public CHeapObj<mtSynchronizer> {
8989
// Debugging fields for naming, deadlock detection, etc. (some only used in debug mode)
9090
#ifndef PRODUCT
9191
bool _allow_vm_block;
92+
#endif
93+
#ifdef ASSERT
9294
int _rank; // rank (to avoid/detect potential deadlocks)
9395
Mutex* _next; // Used by a Thread to link up owned locks
9496
Thread* _last_owner; // the last thread to own the lock
97+
bool _skip_rank_check; // read only by owner when doing rank checks
98+
9599
static bool contains(Mutex* locks, Mutex* lock);
96100
static Mutex* get_least_ranked_lock(Mutex* locks);
97101
Mutex* get_least_ranked_lock_besides_this(Mutex* locks);
98-
#endif // ASSERT
102+
bool skip_rank_check() {
103+
assert(owned_by_self(), "only the owner should call this");
104+
return _skip_rank_check;
105+
}
106+
107+
public:
108+
int rank() const { return _rank; }
109+
Mutex* next() const { return _next; }
110+
void set_next(Mutex *next) { _next = next; }
111+
#endif // ASSERT
99112

113+
protected:
100114
void set_owner_implementation(Thread* owner) NOT_DEBUG({ _owner = owner;});
101115
void check_block_state (Thread* thread) NOT_DEBUG_RETURN;
102116
void check_safepoint_state (Thread* thread) NOT_DEBUG_RETURN;
103117
void check_no_safepoint_state(Thread* thread) NOT_DEBUG_RETURN;
118+
void check_rank (Thread* thread) NOT_DEBUG_RETURN;
104119
void assert_owner (Thread* expected) NOT_DEBUG_RETURN;
105120
void no_safepoint_verifier (Thread* thread, bool enable) NOT_DEBUG_RETURN;
106121

@@ -170,6 +185,7 @@ class Mutex : public CHeapObj<mtSynchronizer> {
170185
bool try_lock(); // Like lock(), but unblocking. It returns false instead
171186
private:
172187
void lock_contended(Thread *thread); // contended slow-path
188+
bool try_lock_inner(bool do_rank_checks);
173189
public:
174190

175191
void release_for_safepoint();
@@ -178,33 +194,25 @@ class Mutex : public CHeapObj<mtSynchronizer> {
178194
// that is guaranteed not to block while running inside the VM.
179195
void lock_without_safepoint_check();
180196
void lock_without_safepoint_check(Thread* self);
197+
// A thread should not call this if failure to acquire ownership will blocks its progress
198+
bool try_lock_without_rank_check();
181199

182200
// Current owner - not not MT-safe. Can only be used to guarantee that
183201
// the current running thread owns the lock
184202
Thread* owner() const { return _owner; }
203+
void set_owner(Thread* owner) { set_owner_implementation(owner); }
185204
bool owned_by_self() const;
186205

187206
const char *name() const { return _name; }
188207

189208
void print_on_error(outputStream* st) const;
190-
191209
#ifndef PRODUCT
192210
void print_on(outputStream* st) const;
193211
void print() const { print_on(::tty); }
194212
#endif
195-
#ifdef ASSERT
196-
int rank() const { return _rank; }
197-
bool allow_vm_block() { return _allow_vm_block; }
198-
199-
Mutex *next() const { return _next; }
200-
void set_next(Mutex *next) { _next = next; }
201-
#endif // ASSERT
202-
203-
void set_owner(Thread* owner) { set_owner_implementation(owner); }
204213
};
205214

206215
class Monitor : public Mutex {
207-
void assert_wait_lock_state (Thread* self) NOT_DEBUG_RETURN;
208216
public:
209217
Monitor(int rank, const char *name, bool allow_vm_block = false,
210218
SafepointCheckRequired safepoint_check_required = _safepoint_check_always);

0 commit comments

Comments
 (0)