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_;