commit 146767f28effd6bda637d4f8ce7b5014326a7dfe
author ShuJieWang <wangshujie.matt@bytedance.com> Wed Dec 22 18:15:58 2021 +0800
committer Treehugger Robot <treehugger-gerrit@google.com> Tue Jan 04 17:45:39 2022 +0000
tree d5b71e6559a64f51afebfac97bdba64768a272df
parent 851b96239f6c7602bc2d2fdd7585b07a0fecca09

Add a new Lock 'env_hooks_lock_' to avoid race conditions

For now, only TI agents would trigger AddEnvironmentHook() at runtime.

Test: art/test.py -b --host
Change-Id: I1d44b93e66a5cd98188bbac31ad21ce7a46923cb

runtime/jni/java_vm_ext.cc

diff --git a/runtime/jni/java_vm_ext.cc b/runtime/jni/java_vm_ext.cc
index dd45c32..12e6d22 100644
--- a/runtime/jni/java_vm_ext.cc
+++ b/runtime/jni/java_vm_ext.cc
@@ -509,6 +509,7 @@
       weak_globals_add_condition_("weak globals add condition",
                                   (CHECK(Locks::jni_weak_globals_lock_ != nullptr),
                                    *Locks::jni_weak_globals_lock_)),
+      env_hooks_lock_("environment hooks lock", art::kGenericBottomLock),
       env_hooks_(),
       enable_allocation_tracking_delta_(
           runtime_options.GetOrDefault(RuntimeArgumentMap::GlobalRefAllocStackTraceLimit)),
@@ -536,7 +537,12 @@
 }
 
 jint JavaVMExt::HandleGetEnv(/*out*/void** env, jint version) {
-  for (GetEnvHook hook : env_hooks_) {
+  std::vector<GetEnvHook> env_hooks;
+  {
+    ReaderMutexLock rmu(Thread::Current(), env_hooks_lock_);
+    env_hooks.assign(env_hooks_.begin(), env_hooks_.end());
+  }
+  for (GetEnvHook hook : env_hooks) {
     jint res = hook(this, env, version);
     if (res == JNI_OK) {
       return JNI_OK;
@@ -552,6 +558,7 @@
 // Add a hook to handle getting environments from the GetEnv call.
 void JavaVMExt::AddEnvironmentHook(GetEnvHook hook) {
   CHECK(hook != nullptr) << "environment hooks shouldn't be null!";
+  WriterMutexLock wmu(Thread::Current(), env_hooks_lock_);
   env_hooks_.push_back(hook);
 }
 

runtime/jni/java_vm_ext.h

diff --git a/runtime/jni/java_vm_ext.h b/runtime/jni/java_vm_ext.h
index a842efa..50e8414 100644
--- a/runtime/jni/java_vm_ext.h
+++ b/runtime/jni/java_vm_ext.h
@@ -200,9 +200,11 @@
   void TrimGlobals() REQUIRES_SHARED(Locks::mutator_lock_)
       REQUIRES(!Locks::jni_globals_lock_);
 
-  jint HandleGetEnv(/*out*/void** env, jint version);
+  jint HandleGetEnv(/*out*/void** env, jint version)
+      REQUIRES(!env_hooks_lock_);
 
-  void AddEnvironmentHook(GetEnvHook hook);
+  void AddEnvironmentHook(GetEnvHook hook)
+      REQUIRES(!env_hooks_lock_);
 
   static bool IsBadJniVersion(int version);
 
@@ -265,7 +267,8 @@
   ConditionVariable weak_globals_add_condition_ GUARDED_BY(Locks::jni_weak_globals_lock_);
 
   // TODO Maybe move this to Runtime.
-  std::vector<GetEnvHook> env_hooks_;
+  ReaderWriterMutex env_hooks_lock_ BOTTOM_MUTEX_ACQUIRED_AFTER;
+  std::vector<GetEnvHook> env_hooks_ GUARDED_BY(env_hooks_lock_);
 
   size_t enable_allocation_tracking_delta_;
   std::atomic<bool> allocation_tracking_enabled_;