ART: Make JNI global and weak global locks global
Add them to Locks. Give the JNI global table lock
a lock level, close to the weak global JNI references.
Bug: 31684578
Test: m test-art-host
Change-Id: I3857a3b0be69b16811d9999096b2c42e7a25d227
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index 1183dea..e77e6d7 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -64,6 +64,8 @@
Mutex* Locks::trace_lock_ = nullptr;
Mutex* Locks::unexpected_signal_lock_ = nullptr;
Uninterruptible Roles::uninterruptible_;
+ReaderWriterMutex* Locks::jni_globals_lock_ = nullptr;
+Mutex* Locks::jni_weak_globals_lock_ = nullptr;
struct AllMutexData {
// A guard for all_mutexes_ that's not a mutex (Mutexes must CAS to acquire and busy wait).
@@ -1088,6 +1090,15 @@
DCHECK(reference_queue_soft_references_lock_ == nullptr);
reference_queue_soft_references_lock_ = new Mutex("ReferenceQueue soft references lock", current_lock_level);
+ UPDATE_CURRENT_LOCK_LEVEL(kJniGlobalsLock);
+ DCHECK(jni_globals_lock_ == nullptr);
+ jni_globals_lock_ =
+ new ReaderWriterMutex("JNI global reference table lock", current_lock_level);
+
+ UPDATE_CURRENT_LOCK_LEVEL(kJniWeakGlobalsLock);
+ DCHECK(jni_weak_globals_lock_ == nullptr);
+ jni_weak_globals_lock_ = new Mutex("JNI weak global reference table lock", current_lock_level);
+
UPDATE_CURRENT_LOCK_LEVEL(kAbortLock);
DCHECK(abort_lock_ == nullptr);
abort_lock_ = new Mutex("abort lock", current_lock_level, true);
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index b3ff6c2..e0cca7b 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -68,6 +68,7 @@
kMarkSweepMarkStackLock,
kTransactionLogLock,
kJniWeakGlobalsLock,
+ kJniGlobalsLock,
kReferenceQueueSoftReferencesLock,
kReferenceQueuePhantomReferencesLock,
kReferenceQueueFinalizerReferencesLock,
@@ -678,8 +679,14 @@
// Guards soft references queue.
static Mutex* reference_queue_soft_references_lock_ ACQUIRED_AFTER(reference_queue_phantom_references_lock_);
+ // Guard accesses to the JNI Global Reference table.
+ static ReaderWriterMutex* jni_globals_lock_ ACQUIRED_AFTER(reference_queue_soft_references_lock_);
+
+ // Guard accesses to the JNI Weak Global Reference table.
+ static Mutex* jni_weak_globals_lock_ ACQUIRED_AFTER(jni_globals_lock_);
+
// Have an exclusive aborting thread.
- static Mutex* abort_lock_ ACQUIRED_AFTER(reference_queue_soft_references_lock_);
+ static Mutex* abort_lock_ ACQUIRED_AFTER(jni_weak_globals_lock_);
// Allow mutual exclusion when manipulating Thread::suspend_count_.
// TODO: Does the trade-off of a per-thread lock make sense?
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index c5f95eb..f2bda05 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -422,14 +422,14 @@
tracing_enabled_(runtime_options.Exists(RuntimeArgumentMap::JniTrace)
|| VLOG_IS_ON(third_party_jni)),
trace_(runtime_options.GetOrDefault(RuntimeArgumentMap::JniTrace)),
- globals_lock_("JNI global reference table lock"),
globals_(gGlobalsInitial, gGlobalsMax, kGlobal),
libraries_(new Libraries),
unchecked_functions_(&gJniInvokeInterface),
- weak_globals_lock_("JNI weak global reference table lock", kJniWeakGlobalsLock),
weak_globals_(kWeakGlobalsInitial, kWeakGlobalsMax, kWeakGlobal),
allow_accessing_weak_globals_(true),
- weak_globals_add_condition_("weak globals add condition", weak_globals_lock_),
+ weak_globals_add_condition_("weak globals add condition",
+ (CHECK(Locks::jni_weak_globals_lock_ != nullptr),
+ *Locks::jni_weak_globals_lock_)),
env_hooks_() {
functions = unchecked_functions_;
SetCheckJniEnabled(runtime_options.Exists(RuntimeArgumentMap::CheckJni));
@@ -537,7 +537,7 @@
if (obj == nullptr) {
return nullptr;
}
- WriterMutexLock mu(self, globals_lock_);
+ WriterMutexLock mu(self, *Locks::jni_globals_lock_);
IndirectRef ref = globals_.Add(IRT_FIRST_SEGMENT, obj);
return reinterpret_cast<jobject>(ref);
}
@@ -546,7 +546,7 @@
if (obj == nullptr) {
return nullptr;
}
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
while (UNLIKELY(!MayAccessWeakGlobals(self))) {
weak_globals_add_condition_.WaitHoldingLocks(self);
}
@@ -558,7 +558,7 @@
if (obj == nullptr) {
return;
}
- WriterMutexLock mu(self, globals_lock_);
+ WriterMutexLock mu(self, *Locks::jni_globals_lock_);
if (!globals_.Remove(IRT_FIRST_SEGMENT, obj)) {
LOG(WARNING) << "JNI WARNING: DeleteGlobalRef(" << obj << ") "
<< "failed to find entry";
@@ -569,7 +569,7 @@
if (obj == nullptr) {
return;
}
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
if (!weak_globals_.Remove(IRT_FIRST_SEGMENT, obj)) {
LOG(WARNING) << "JNI WARNING: DeleteWeakGlobalRef(" << obj << ") "
<< "failed to find entry";
@@ -597,11 +597,11 @@
}
Thread* self = Thread::Current();
{
- ReaderMutexLock mu(self, globals_lock_);
+ ReaderMutexLock mu(self, *Locks::jni_globals_lock_);
os << "; globals=" << globals_.Capacity();
}
{
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
if (weak_globals_.Capacity() > 0) {
os << " (plus " << weak_globals_.Capacity() << " weak)";
}
@@ -617,7 +617,7 @@
void JavaVMExt::DisallowNewWeakGlobals() {
CHECK(!kUseReadBarrier);
Thread* const self = Thread::Current();
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
// DisallowNewWeakGlobals is only called by CMS during the pause. It is required to have the
// mutator lock exclusively held so that we don't have any threads in the middle of
// DecodeWeakGlobal.
@@ -628,7 +628,7 @@
void JavaVMExt::AllowNewWeakGlobals() {
CHECK(!kUseReadBarrier);
Thread* self = Thread::Current();
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
allow_accessing_weak_globals_.StoreSequentiallyConsistent(true);
weak_globals_add_condition_.Broadcast(self);
}
@@ -636,7 +636,7 @@
void JavaVMExt::BroadcastForNewWeakGlobals() {
CHECK(kUseReadBarrier);
Thread* self = Thread::Current();
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
weak_globals_add_condition_.Broadcast(self);
}
@@ -645,7 +645,7 @@
}
void JavaVMExt::UpdateGlobal(Thread* self, IndirectRef ref, ObjPtr<mirror::Object> result) {
- WriterMutexLock mu(self, globals_lock_);
+ WriterMutexLock mu(self, *Locks::jni_globals_lock_);
globals_.Update(ref, result);
}
@@ -671,13 +671,13 @@
if (LIKELY(MayAccessWeakGlobalsUnlocked(self))) {
return weak_globals_.SynchronizedGet(ref);
}
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
return DecodeWeakGlobalLocked(self, ref);
}
ObjPtr<mirror::Object> JavaVMExt::DecodeWeakGlobalLocked(Thread* self, IndirectRef ref) {
if (kDebugLocking) {
- weak_globals_lock_.AssertHeld(self);
+ Locks::jni_weak_globals_lock_->AssertHeld(self);
}
while (UNLIKELY(!MayAccessWeakGlobals(self))) {
weak_globals_add_condition_.WaitHoldingLocks(self);
@@ -700,7 +700,7 @@
bool JavaVMExt::IsWeakGlobalCleared(Thread* self, IndirectRef ref) {
DCHECK_EQ(GetIndirectRefKind(ref), kWeakGlobal);
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
while (UNLIKELY(!MayAccessWeakGlobals(self))) {
weak_globals_add_condition_.WaitHoldingLocks(self);
}
@@ -712,18 +712,18 @@
}
void JavaVMExt::UpdateWeakGlobal(Thread* self, IndirectRef ref, ObjPtr<mirror::Object> result) {
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
weak_globals_.Update(ref, result);
}
void JavaVMExt::DumpReferenceTables(std::ostream& os) {
Thread* self = Thread::Current();
{
- ReaderMutexLock mu(self, globals_lock_);
+ ReaderMutexLock mu(self, *Locks::jni_globals_lock_);
globals_.Dump(os);
}
{
- MutexLock mu(self, weak_globals_lock_);
+ MutexLock mu(self, *Locks::jni_weak_globals_lock_);
weak_globals_.Dump(os);
}
}
@@ -920,7 +920,7 @@
}
void JavaVMExt::SweepJniWeakGlobals(IsMarkedVisitor* visitor) {
- MutexLock mu(Thread::Current(), weak_globals_lock_);
+ MutexLock mu(Thread::Current(), *Locks::jni_weak_globals_lock_);
Runtime* const runtime = Runtime::Current();
for (auto* entry : weak_globals_) {
// Need to skip null here to distinguish between null entries and cleared weak ref entries.
@@ -937,13 +937,13 @@
}
void JavaVMExt::TrimGlobals() {
- WriterMutexLock mu(Thread::Current(), globals_lock_);
+ WriterMutexLock mu(Thread::Current(), *Locks::jni_globals_lock_);
globals_.Trim();
}
void JavaVMExt::VisitRoots(RootVisitor* visitor) {
Thread* self = Thread::Current();
- ReaderMutexLock mu(self, globals_lock_);
+ ReaderMutexLock mu(self, *Locks::jni_globals_lock_);
globals_.VisitRoots(visitor, RootInfo(kRootJNIGlobal));
// The weak_globals table is visited by the GC itself (because it mutates the table).
}
diff --git a/runtime/java_vm_ext.h b/runtime/java_vm_ext.h
index 2e59a9d..05717f4 100644
--- a/runtime/java_vm_ext.h
+++ b/runtime/java_vm_ext.h
@@ -109,72 +109,81 @@
REQUIRES_SHARED(Locks::mutator_lock_);
void DumpForSigQuit(std::ostream& os)
- REQUIRES(!Locks::jni_libraries_lock_, !globals_lock_, !weak_globals_lock_);
+ REQUIRES(!Locks::jni_libraries_lock_,
+ !Locks::jni_globals_lock_,
+ !Locks::jni_weak_globals_lock_);
void DumpReferenceTables(std::ostream& os)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!globals_lock_, !weak_globals_lock_);
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_globals_lock_, !Locks::jni_weak_globals_lock_);
bool SetCheckJniEnabled(bool enabled);
void VisitRoots(RootVisitor* visitor) REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!globals_lock_);
+ REQUIRES(!Locks::jni_globals_lock_);
- void DisallowNewWeakGlobals() REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!weak_globals_lock_);
- void AllowNewWeakGlobals() REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!weak_globals_lock_);
- void BroadcastForNewWeakGlobals() REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!weak_globals_lock_);
+ void DisallowNewWeakGlobals()
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_weak_globals_lock_);
+ void AllowNewWeakGlobals()
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_weak_globals_lock_);
+ void BroadcastForNewWeakGlobals()
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_weak_globals_lock_);
jobject AddGlobalRef(Thread* self, ObjPtr<mirror::Object> obj)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!globals_lock_);
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_globals_lock_);
jweak AddWeakGlobalRef(Thread* self, ObjPtr<mirror::Object> obj)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!weak_globals_lock_);
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_weak_globals_lock_);
- void DeleteGlobalRef(Thread* self, jobject obj) REQUIRES(!globals_lock_);
+ void DeleteGlobalRef(Thread* self, jobject obj) REQUIRES(!Locks::jni_globals_lock_);
- void DeleteWeakGlobalRef(Thread* self, jweak obj) REQUIRES(!weak_globals_lock_);
+ void DeleteWeakGlobalRef(Thread* self, jweak obj) REQUIRES(!Locks::jni_weak_globals_lock_);
void SweepJniWeakGlobals(IsMarkedVisitor* visitor)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!weak_globals_lock_);
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_weak_globals_lock_);
ObjPtr<mirror::Object> DecodeGlobal(IndirectRef ref)
REQUIRES_SHARED(Locks::mutator_lock_);
void UpdateGlobal(Thread* self, IndirectRef ref, ObjPtr<mirror::Object> result)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!globals_lock_);
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_globals_lock_);
ObjPtr<mirror::Object> DecodeWeakGlobal(Thread* self, IndirectRef ref)
REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!weak_globals_lock_);
+ REQUIRES(!Locks::jni_weak_globals_lock_);
ObjPtr<mirror::Object> DecodeWeakGlobalLocked(Thread* self, IndirectRef ref)
REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(weak_globals_lock_);
+ REQUIRES(Locks::jni_weak_globals_lock_);
// Like DecodeWeakGlobal() but to be used only during a runtime shutdown where self may be
// null.
ObjPtr<mirror::Object> DecodeWeakGlobalDuringShutdown(Thread* self, IndirectRef ref)
REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!weak_globals_lock_);
+ REQUIRES(!Locks::jni_weak_globals_lock_);
// Checks if the weak global ref has been cleared by the GC without decode (read barrier.)
bool IsWeakGlobalCleared(Thread* self, IndirectRef ref)
REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!weak_globals_lock_);
-
- Mutex& WeakGlobalsLock() RETURN_CAPABILITY(weak_globals_lock_) {
- return weak_globals_lock_;
- }
+ REQUIRES(!Locks::jni_weak_globals_lock_);
void UpdateWeakGlobal(Thread* self, IndirectRef ref, ObjPtr<mirror::Object> result)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!weak_globals_lock_);
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Locks::jni_weak_globals_lock_);
const JNIInvokeInterface* GetUncheckedFunctions() const {
return unchecked_functions_;
}
void TrimGlobals() REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!globals_lock_);
+ REQUIRES(!Locks::jni_globals_lock_);
jint HandleGetEnv(/*out*/void** env, jint version);
@@ -187,7 +196,7 @@
bool MayAccessWeakGlobalsUnlocked(Thread* self) const REQUIRES_SHARED(Locks::mutator_lock_);
bool MayAccessWeakGlobals(Thread* self) const
REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(weak_globals_lock_);
+ REQUIRES(Locks::jni_weak_globals_lock_);
Runtime* const runtime_;
@@ -203,8 +212,6 @@
// Extra diagnostics.
const std::string trace_;
- // JNI global references.
- ReaderWriterMutex globals_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
// Not guarded by globals_lock since we sometimes use SynchronizedGet in Thread::DecodeJObject.
IndirectReferenceTable globals_;
@@ -215,8 +222,6 @@
// Used by -Xcheck:jni.
const JNIInvokeInterface* const unchecked_functions_;
- // JNI weak global references.
- Mutex weak_globals_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
// Since weak_globals_ contain weak roots, be careful not to
// directly access the object references in it. Use Get() with the
// read barrier enabled.
@@ -224,7 +229,7 @@
IndirectReferenceTable weak_globals_;
// Not guarded by weak_globals_lock since we may use SynchronizedGet in DecodeWeakGlobal.
Atomic<bool> allow_accessing_weak_globals_;
- ConditionVariable weak_globals_add_condition_ GUARDED_BY(weak_globals_lock_);
+ ConditionVariable weak_globals_add_condition_ GUARDED_BY(Locks::jni_weak_globals_lock_);
// TODO Maybe move this to Runtime.
std::vector<GetEnvHook> env_hooks_;