Use try lock to fix class resolution race
There was some possible deadlocks related to EnsureResolved caused by
acquiring an object lock.
Scenario:
Thread 1 acquires lock on obj1
Thread 1 begins to resolve / initialize class1
Thread 1 blocks since it sees that class1 is already being resolved and
gets preempted before it can acquire the object lock on class1
Thread 2 finishes resolving and initializing class1 and locks class1
Thread 2 blocks attempting to lock obj1
Thread 1 blocks attempting to lock class1
Deadlock
Fixed the deadlock by changing EnsureResolved to use a try lock for the
unresolved case.
Added a test.
Test: Device boot, test-art-host, monitor_test
Bug: 27417671
(cherry picked from commit a704eda0078989a73cac111ed309aca50d2e289b)
Change-Id: I1150b19bdc1a5cc87ae95eda4f2b6b4bca215a60
diff --git a/runtime/monitor.cc b/runtime/monitor.cc
index 0f56705..3771877 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.