Revert "Revert "Use try lock to fix class resolution race""
Fix possible deadlock in EnsureResolved caused by interaction with
GC. Since we were sleeping while holding the mutator lock, it could
block thread suspension. This would deadlock if the thread that
had locked h_class is already suspended since we would spin forever
and not make progress.
Bug: 27417671
Bug: 30500547
Test: test-art-host ART_TEST_GC_STRESS=true
This reverts commit 69bf969c055c31a75d17ea92aeee756042678114.
Change-Id: Id8750df065dc3b9ef7dc874f2eb2cc2c58e5d1eb
diff --git a/runtime/monitor.cc b/runtime/monitor.cc
index 396c946..bf9f931 100644
--- a/runtime/monitor.cc
+++ b/runtime/monitor.cc
@@ -314,21 +314,34 @@
return oss.str();
}
+bool Monitor::TryLockLocked(Thread* self) {
+ if (owner_ == nullptr) { // Unowned.
+ owner_ = self;
+ CHECK_EQ(lock_count_, 0);
+ // When debugging, save the current monitor holder for future
+ // acquisition failures to use in sampled logging.
+ if (lock_profiling_threshold_ != 0) {
+ locking_method_ = self->GetCurrentMethod(&locking_dex_pc_);
+ }
+ } else if (owner_ == self) { // Recursive.
+ lock_count_++;
+ } else {
+ return false;
+ }
+ AtraceMonitorLock(self, GetObject(), false /* is_wait */);
+ return true;
+}
+
+bool Monitor::TryLock(Thread* self) {
+ MutexLock mu(self, monitor_lock_);
+ return TryLockLocked(self);
+}
+
void Monitor::Lock(Thread* self) {
MutexLock mu(self, monitor_lock_);
while (true) {
- if (owner_ == nullptr) { // Unowned.
- owner_ = self;
- CHECK_EQ(lock_count_, 0);
- // When debugging, save the current monitor holder for future
- // acquisition failures to use in sampled logging.
- if (lock_profiling_threshold_ != 0) {
- locking_method_ = self->GetCurrentMethod(&locking_dex_pc_);
- }
- break;
- } else if (owner_ == self) { // Recursive.
- lock_count_++;
- break;
+ if (TryLockLocked(self)) {
+ return;
}
// Contended.
const bool log_contention = (lock_profiling_threshold_ != 0);
@@ -430,8 +443,6 @@
monitor_lock_.Lock(self); // Reacquire locks in order.
--num_waiters_;
}
-
- AtraceMonitorLock(self, GetObject(), false /* is_wait */);
}
static void ThrowIllegalMonitorStateExceptionF(const char* fmt, ...)
@@ -852,7 +863,7 @@
return obj;
}
-mirror::Object* Monitor::MonitorEnter(Thread* self, mirror::Object* obj) {
+mirror::Object* Monitor::MonitorEnter(Thread* self, mirror::Object* obj, bool trylock) {
DCHECK(self != nullptr);
DCHECK(obj != nullptr);
self->AssertThreadSuspensionIsAllowable();
@@ -898,6 +909,9 @@
InflateThinLocked(self, h_obj, lock_word, 0);
}
} else {
+ if (trylock) {
+ return nullptr;
+ }
// Contention.
contention_count++;
Runtime* runtime = Runtime::Current();
@@ -916,8 +930,12 @@
}
case LockWord::kFatLocked: {
Monitor* mon = lock_word.FatLockMonitor();
- mon->Lock(self);
- return h_obj.Get(); // Success!
+ if (trylock) {
+ return mon->TryLock(self) ? h_obj.Get() : nullptr;
+ } else {
+ mon->Lock(self);
+ return h_obj.Get(); // Success!
+ }
}
case LockWord::kHashCode:
// Inflate with the existing hashcode.