commit b284f8d775ac32d8109744d94b99da451570beef
author Alex Light <allight@google.com> Tue Nov 21 00:00:48 2017 +0000
committer Alex Light <allight@google.com> Mon Nov 20 16:58:05 2017 -0800
tree c720648f1c018d224410314b30a891f2752bc781
parent e6779fde5a3c0a8bb484a61398da439a37d25b92

Revert "Revert "Make JVMTI DisposeEnvironment and GetEnv thread safe.""

This reverts commit af9341087aab0146b8323ece156bde8130948465.

We needed to allow TopLockLevel locks to be acquired when the
mutator_lock_ is exclusive held. This is required for spec
conformance. To ensure there are no deadlocks the mutator_lock_ is the
only lock level with this exception and one cannot acquire the
mutator_lock_ when one holds any kTopLockLevel locks.

Reason for revert: Fixed issue causing test 913 failure in art-gc-gss-tlab
Test: ART_DEFAULT_GC_TYPE=GSS \
      ART_USE_TLAB=true \
      ART_USE_READ_BARRIER=false
      ./test.py --host -j50
Bug: 69465262

Change-Id: Ic1a4d9bb3ff64382ba7ae22ba27a4f44628ed095

openjdkjvmti/events.cc

diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc
index d1d606d..be4ebbc 100644
--- a/openjdkjvmti/events.cc
+++ b/openjdkjvmti/events.cc
@@ -193,25 +193,21 @@
 }
 
 void EventHandler::RegisterArtJvmTiEnv(ArtJvmTiEnv* env) {
-  // Since we never shrink this array we might as well try to fill gaps.
-  auto it = std::find(envs.begin(), envs.end(), nullptr);
-  if (it != envs.end()) {
-    *it = env;
-  } else {
-    envs.push_back(env);
-  }
+  art::MutexLock mu(art::Thread::Current(), envs_lock_);
+  envs.push_back(env);
 }
 
 void EventHandler::RemoveArtJvmTiEnv(ArtJvmTiEnv* env) {
+  art::MutexLock mu(art::Thread::Current(), envs_lock_);
   // Since we might be currently iterating over the envs list we cannot actually erase elements.
   // Instead we will simply replace them with 'nullptr' and skip them manually.
   auto it = std::find(envs.begin(), envs.end(), env);
   if (it != envs.end()) {
-    *it = nullptr;
+    envs.erase(it);
     for (size_t i = static_cast<size_t>(ArtJvmtiEvent::kMinEventTypeVal);
          i <= static_cast<size_t>(ArtJvmtiEvent::kMaxEventTypeVal);
          ++i) {
-      RecalculateGlobalEventMask(static_cast<ArtJvmtiEvent>(i));
+      RecalculateGlobalEventMaskLocked(static_cast<ArtJvmtiEvent>(i));
     }
   }
 }
@@ -431,11 +427,11 @@
         finish_enabled_(false) {}
 
   void StartPause() OVERRIDE {
-    handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionStart>(nullptr);
+    handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionStart>(art::Thread::Current());
   }
 
   void EndPause() OVERRIDE {
-    handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionFinish>(nullptr);
+    handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionFinish>(art::Thread::Current());
   }
 
   bool IsEnabled() {
@@ -1176,7 +1172,8 @@
   art::Runtime::Current()->GetInstrumentation()->RemoveListener(method_trace_listener_.get(), ~0);
 }
 
-EventHandler::EventHandler() {
+EventHandler::EventHandler() : envs_lock_("JVMTI Environment List Lock",
+                                          art::LockLevel::kTopLockLevel) {
   alloc_listener_.reset(new JvmtiAllocationListener(this));
   ddm_listener_.reset(new JvmtiDdmChunkListener(this));
   gc_pause_listener_.reset(new JvmtiGcPauseListener(this));