Merge "ART: Do not run verification under lock" am: a455ec2

am: ff5d050

* commit 'ff5d050631ff042283a18bfa37bec0057db2b450':
  ART: Do not run verification under lock

Change-Id: Ibbbd44a0d6121ba4464a092a48ac15baeacaf5ec

1 files changed, 47 insertions, 33 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 876eecdaea..9d26d1305c 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3742,10 +3742,6 @@ bool ClassLinker::AttemptSupertypeVerification(Thread* self,
   DCHECK(klass.Get() != nullptr);
   DCHECK(supertype.Get() != nullptr);
 
-  StackHandleScope<1> hs(self);
-  // Acquire lock to prevent races on verifying the super class.
-  ObjectLock<mirror::Class> super_lock(self, supertype);
-
   if (!supertype->IsVerified() && !supertype->IsErroneous()) {
     VerifyClass(self, supertype);
   }
@@ -3759,6 +3755,7 @@ bool ClassLinker::AttemptSupertypeVerification(Thread* self,
                    PrettyDescriptor(klass.Get()).c_str(),
                    PrettyDescriptor(supertype.Get()).c_str());
   LOG(WARNING) << error_msg  << " in " << klass->GetDexCache()->GetLocation()->ToModifiedUtf8();
+  StackHandleScope<1> hs(self);
   Handle<mirror::Throwable> cause(hs.NewHandle(self->GetException()));
   if (cause.Get() != nullptr) {
     // Set during VerifyClass call (if at all).
@@ -3773,44 +3770,57 @@ bool ClassLinker::AttemptSupertypeVerification(Thread* self,
   if (Runtime::Current()->IsAotCompiler()) {
     Runtime::Current()->GetCompilerCallbacks()->ClassRejected(ref);
   }
+  // Need to grab the lock to change status.
+  ObjectLock<mirror::Class> super_lock(self, klass);
   mirror::Class::SetStatus(klass, mirror::Class::kStatusError, self);
   return false;
 }
 
 void ClassLinker::VerifyClass(Thread* self, Handle<mirror::Class> klass, LogSeverity log_level) {
-  // TODO: assert that the monitor on the Class is held
-  ObjectLock<mirror::Class> lock(self, klass);
+  {
+    // TODO: assert that the monitor on the Class is held
+    ObjectLock<mirror::Class> lock(self, klass);
 
-  // Don't attempt to re-verify if already sufficiently verified.
-  if (klass->IsVerified()) {
-    EnsureSkipAccessChecksMethods(klass);
-    return;
-  }
-  if (klass->IsCompileTimeVerified() && Runtime::Current()->IsAotCompiler()) {
-    return;
-  }
+    // Is somebody verifying this now?
+    mirror::Class::Status old_status = klass->GetStatus();
+    while (old_status == mirror::Class::kStatusVerifying ||
+        old_status == mirror::Class::kStatusVerifyingAtRuntime) {
+      lock.WaitIgnoringInterrupts();
+      CHECK_GT(klass->GetStatus(), old_status);
+      old_status = klass->GetStatus();
+    }
 
-  // The class might already be erroneous, for example at compile time if we attempted to verify
-  // this class as a parent to another.
-  if (klass->IsErroneous()) {
-    ThrowEarlierClassFailure(klass.Get());
-    return;
-  }
+    // The class might already be erroneous, for example at compile time if we attempted to verify
+    // this class as a parent to another.
+    if (klass->IsErroneous()) {
+      ThrowEarlierClassFailure(klass.Get());
+      return;
+    }
 
-  if (klass->GetStatus() == mirror::Class::kStatusResolved) {
-    mirror::Class::SetStatus(klass, mirror::Class::kStatusVerifying, self);
-  } else {
-    CHECK_EQ(klass->GetStatus(), mirror::Class::kStatusRetryVerificationAtRuntime)
-        << PrettyClass(klass.Get());
-    CHECK(!Runtime::Current()->IsAotCompiler());
-    mirror::Class::SetStatus(klass, mirror::Class::kStatusVerifyingAtRuntime, self);
-  }
+    // Don't attempt to re-verify if already sufficiently verified.
+    if (klass->IsVerified()) {
+      EnsureSkipAccessChecksMethods(klass);
+      return;
+    }
+    if (klass->IsCompileTimeVerified() && Runtime::Current()->IsAotCompiler()) {
+      return;
+    }
 
-  // Skip verification if disabled.
-  if (!Runtime::Current()->IsVerificationEnabled()) {
-    mirror::Class::SetStatus(klass, mirror::Class::kStatusVerified, self);
-    EnsureSkipAccessChecksMethods(klass);
-    return;
+    if (klass->GetStatus() == mirror::Class::kStatusResolved) {
+      mirror::Class::SetStatus(klass, mirror::Class::kStatusVerifying, self);
+    } else {
+      CHECK_EQ(klass->GetStatus(), mirror::Class::kStatusRetryVerificationAtRuntime)
+            << PrettyClass(klass.Get());
+      CHECK(!Runtime::Current()->IsAotCompiler());
+      mirror::Class::SetStatus(klass, mirror::Class::kStatusVerifyingAtRuntime, self);
+    }
+
+    // Skip verification if disabled.
+    if (!Runtime::Current()->IsVerificationEnabled()) {
+      mirror::Class::SetStatus(klass, mirror::Class::kStatusVerified, self);
+      EnsureSkipAccessChecksMethods(klass);
+      return;
+    }
   }
 
   // Verify super class.
@@ -3883,6 +3893,10 @@ void ClassLinker::VerifyClass(Thread* self, Handle<mirror::Class> klass, LogSeve
                                                              log_level,
                                                              &error_msg);
   }
+
+  // Verification is done, grab the lock again.
+  ObjectLock<mirror::Class> lock(self, klass);
+
   if (preverified || verifier_failure != verifier::MethodVerifier::kHardFailure) {
     if (!preverified && verifier_failure != verifier::MethodVerifier::kNoFailure) {
       VLOG(class_linker) << "Soft verification failure in class " << PrettyDescriptor(klass.Get())