Add two locks to expected_mutexes_on_weak_ref_access_.
The ObjectRegistry lock and the jdwp event list lock to avoid DCHECK
failures.
Some cleanup.
Bug: 35360959
Bug: 35745310
Test: test-art-host
Test: jdwp test.
Test: angler boot.
Change-Id: I16000c0c7624b1271d40c4c1a01a4e249271b67e
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index b93b293..b0394a5 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -73,6 +73,7 @@
Mutex* Locks::jni_weak_globals_lock_ = nullptr;
ReaderWriterMutex* Locks::dex_lock_ = nullptr;
std::vector<BaseMutex*> Locks::expected_mutexes_on_weak_ref_access_;
+Atomic<const BaseMutex*> Locks::expected_mutexes_on_weak_ref_access_guard_;
struct AllMutexData {
// A guard for all_mutexes_ that's not a mutex (Mutexes must CAS to acquire and busy wait).
@@ -117,6 +118,26 @@
const BaseMutex* const mutex_;
};
+class Locks::ScopedExpectedMutexesOnWeakRefAccessLock FINAL {
+ public:
+ explicit ScopedExpectedMutexesOnWeakRefAccessLock(const BaseMutex* mutex) : mutex_(mutex) {
+ while (!Locks::expected_mutexes_on_weak_ref_access_guard_.CompareExchangeWeakAcquire(0,
+ mutex)) {
+ NanoSleep(100);
+ }
+ }
+
+ ~ScopedExpectedMutexesOnWeakRefAccessLock() {
+ while (!Locks::expected_mutexes_on_weak_ref_access_guard_.CompareExchangeWeakRelease(mutex_,
+ 0)) {
+ NanoSleep(100);
+ }
+ }
+
+ private:
+ const BaseMutex* const mutex_;
+};
+
// Scoped class that generates events at the beginning and end of lock contention.
class ScopedContentionRecorder FINAL : public ValueObject {
public:
@@ -1163,12 +1184,9 @@
#undef UPDATE_CURRENT_LOCK_LEVEL
// List of mutexes that we may hold when accessing a weak ref.
- dex_lock_->SetShouldRespondToEmptyCheckpointRequest(true);
- expected_mutexes_on_weak_ref_access_.push_back(dex_lock_);
- classlinker_classes_lock_->SetShouldRespondToEmptyCheckpointRequest(true);
- expected_mutexes_on_weak_ref_access_.push_back(classlinker_classes_lock_);
- jni_libraries_lock_->SetShouldRespondToEmptyCheckpointRequest(true);
- expected_mutexes_on_weak_ref_access_.push_back(jni_libraries_lock_);
+ AddToExpectedMutexesOnWeakRefAccess(dex_lock_, /*need_lock*/ false);
+ AddToExpectedMutexesOnWeakRefAccess(classlinker_classes_lock_, /*need_lock*/ false);
+ AddToExpectedMutexesOnWeakRefAccess(jni_libraries_lock_, /*need_lock*/ false);
InitConditions();
}
@@ -1188,4 +1206,38 @@
return safe_to_call_abort_cb != nullptr && safe_to_call_abort_cb();
}
+void Locks::AddToExpectedMutexesOnWeakRefAccess(BaseMutex* mutex, bool need_lock) {
+ if (need_lock) {
+ ScopedExpectedMutexesOnWeakRefAccessLock mu(mutex);
+ mutex->SetShouldRespondToEmptyCheckpointRequest(true);
+ expected_mutexes_on_weak_ref_access_.push_back(mutex);
+ } else {
+ mutex->SetShouldRespondToEmptyCheckpointRequest(true);
+ expected_mutexes_on_weak_ref_access_.push_back(mutex);
+ }
+}
+
+void Locks::RemoveFromExpectedMutexesOnWeakRefAccess(BaseMutex* mutex, bool need_lock) {
+ if (need_lock) {
+ ScopedExpectedMutexesOnWeakRefAccessLock mu(mutex);
+ mutex->SetShouldRespondToEmptyCheckpointRequest(false);
+ std::vector<BaseMutex*>& list = expected_mutexes_on_weak_ref_access_;
+ auto it = std::find(list.begin(), list.end(), mutex);
+ DCHECK(it != list.end());
+ list.erase(it);
+ } else {
+ mutex->SetShouldRespondToEmptyCheckpointRequest(false);
+ std::vector<BaseMutex*>& list = expected_mutexes_on_weak_ref_access_;
+ auto it = std::find(list.begin(), list.end(), mutex);
+ DCHECK(it != list.end());
+ list.erase(it);
+ }
+}
+
+bool Locks::IsExpectedOnWeakRefAccess(BaseMutex* mutex) {
+ ScopedExpectedMutexesOnWeakRefAccessLock mu(mutex);
+ std::vector<BaseMutex*>& list = expected_mutexes_on_weak_ref_access_;
+ return std::find(list.begin(), list.end(), mutex) != list.end();
+}
+
} // namespace art
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index 9b6938f..83fe696 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -583,6 +583,12 @@
// Checks for whether it is safe to call Abort() without using locks.
static bool IsSafeToCallAbortRacy() NO_THREAD_SAFETY_ANALYSIS;
+ // Add a mutex to expected_mutexes_on_weak_ref_access_.
+ static void AddToExpectedMutexesOnWeakRefAccess(BaseMutex* mutex, bool need_lock = true);
+ // Remove a mutex from expected_mutexes_on_weak_ref_access_.
+ static void RemoveFromExpectedMutexesOnWeakRefAccess(BaseMutex* mutex, bool need_lock = true);
+ // Check if the given mutex is in expected_mutexes_on_weak_ref_access_.
+ static bool IsExpectedOnWeakRefAccess(BaseMutex* mutex);
// Guards allocation entrypoint instrumenting.
static Mutex* instrument_entrypoints_lock_;
@@ -734,6 +740,8 @@
// encounter an unexpected mutex on accessing weak refs,
// Thread::CheckEmptyCheckpointFromWeakRefAccess will detect it.
static std::vector<BaseMutex*> expected_mutexes_on_weak_ref_access_;
+ static Atomic<const BaseMutex*> expected_mutexes_on_weak_ref_access_guard_;
+ class ScopedExpectedMutexesOnWeakRefAccessLock;
};
class Roles {
diff --git a/runtime/jdwp/jdwp_main.cc b/runtime/jdwp/jdwp_main.cc
index 7707ba4..e6c6068 100644
--- a/runtime/jdwp/jdwp_main.cc
+++ b/runtime/jdwp/jdwp_main.cc
@@ -239,6 +239,7 @@
shutdown_lock_("JDWP shutdown lock", kJdwpShutdownLock),
shutdown_cond_("JDWP shutdown condition variable", shutdown_lock_),
processing_request_(false) {
+ Locks::AddToExpectedMutexesOnWeakRefAccess(&event_list_lock_);
}
/*
@@ -381,6 +382,8 @@
CHECK(netState == nullptr);
ResetState();
+
+ Locks::RemoveFromExpectedMutexesOnWeakRefAccess(&event_list_lock_);
}
/*
diff --git a/runtime/jdwp/object_registry.cc b/runtime/jdwp/object_registry.cc
index bd7251b..510f5f0 100644
--- a/runtime/jdwp/object_registry.cc
+++ b/runtime/jdwp/object_registry.cc
@@ -35,6 +35,11 @@
ObjectRegistry::ObjectRegistry()
: lock_("ObjectRegistry lock", kJdwpObjectRegistryLock), next_id_(1) {
+ Locks::AddToExpectedMutexesOnWeakRefAccess(&lock_);
+}
+
+ObjectRegistry::~ObjectRegistry() {
+ Locks::RemoveFromExpectedMutexesOnWeakRefAccess(&lock_);
}
JDWP::RefTypeId ObjectRegistry::AddRefType(ObjPtr<mirror::Class> c) {
diff --git a/runtime/jdwp/object_registry.h b/runtime/jdwp/object_registry.h
index 9cacc66..8754631 100644
--- a/runtime/jdwp/object_registry.h
+++ b/runtime/jdwp/object_registry.h
@@ -62,6 +62,7 @@
class ObjectRegistry {
public:
ObjectRegistry();
+ ~ObjectRegistry();
JDWP::ObjectId Add(ObjPtr<mirror::Object> o)
REQUIRES_SHARED(Locks::mutator_lock_)
diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h
index 482e0e3..02a1e4d 100644
--- a/runtime/thread-inl.h
+++ b/runtime/thread-inl.h
@@ -94,9 +94,7 @@
if (held_mutex != nullptr &&
held_mutex != Locks::mutator_lock_ &&
held_mutex != cond_var_mutex) {
- std::vector<BaseMutex*>& expected_mutexes = Locks::expected_mutexes_on_weak_ref_access_;
- CHECK(std::find(expected_mutexes.begin(), expected_mutexes.end(), held_mutex) !=
- expected_mutexes.end())
+ CHECK(Locks::IsExpectedOnWeakRefAccess(held_mutex))
<< "Holding unexpected mutex " << held_mutex->GetName()
<< " when accessing weak ref";
}