commit b087bb226d65a6b1a62102694168b9cdc5b3f665
author ShuJieWang <wangshujie.matt@bytedance.com> Wed Nov 24 11:46:16 2021 +0800
committer Treehugger Robot <treehugger-gerrit@google.com> Mon Dec 13 15:50:31 2021 +0000
tree 05a1a9fcb5224b8a8acf9c8a34c0af1fcd2b254b
parent c2b3116e81fbf465088f775613c62eaf61a5644f

Abort redefinition if we failed to acquire class object

When there are mutiple redefinitions at a time, which require
holding corresponding class objects on creation (without suspension),
may cause a deadlock with other threads in which we might wait on
those objects as well.

Example:
Thread 1(redefiner):  holds class A(on redefinition's creation) -> waits on class B(on another creation)
Thread 2           :  holds class B(through monitor instrs)     -> waits on class A(at following monitor instrs)

Test: art/test.py -b --host

Change-Id: I706fdae5304b36ec1e580040cf81a42f898fe09c

openjdkjvmti/ti_redefine.cc

diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index 80da1b3..44eac47 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -538,11 +538,11 @@
       dex_file_(redefined_dex_file),
       class_sig_(class_sig),
       original_dex_file_(orig_dex_file) {
-  GetMirrorClass()->MonitorEnter(driver_->self_);
+  lock_acquired_ = GetMirrorClass()->MonitorTryEnter(driver_->self_) != nullptr;
 }
 
 Redefiner::ClassRedefinition::~ClassRedefinition() {
-  if (driver_ != nullptr) {
+  if (driver_ != nullptr && lock_acquired_) {
     GetMirrorClass()->MonitorExit(driver_->self_);
   }
 }
@@ -1079,6 +1079,15 @@
   // Get the class as it is now.
   art::Handle<art::mirror::Class> current_class(hs.NewHandle(GetMirrorClass()));
 
+  // Check whether the class object has been successfully acquired.
+  if (!lock_acquired_) {
+      std::string storage;
+      RecordFailure(ERR(INTERNAL),
+                    StringPrintf("Failed to lock class object '%s'",
+                                 current_class->GetDescriptor(&storage)));
+      return false;
+  }
+
   // Check the access flags didn't change.
   if (def.GetJavaAccessFlags() != (current_class->GetAccessFlags() & art::kAccValidClassFlags)) {
     RecordFailure(ERR(UNSUPPORTED_REDEFINITION_CLASS_MODIFIERS_CHANGED),

openjdkjvmti/ti_redefine.h

diff --git a/openjdkjvmti/ti_redefine.h b/openjdkjvmti/ti_redefine.h
index 76de9a7..85b1070 100644
--- a/openjdkjvmti/ti_redefine.h
+++ b/openjdkjvmti/ti_redefine.h
@@ -132,6 +132,7 @@
       dex_file_ = std::move(other.dex_file_);
       class_sig_ = std::move(other.class_sig_);
       original_dex_file_ = other.original_dex_file_;
+      lock_acquired_ = other.lock_acquired_;
       other.driver_ = nullptr;
       return *this;
     }
@@ -142,7 +143,8 @@
           klass_(other.klass_),
           dex_file_(std::move(other.dex_file_)),
           class_sig_(std::move(other.class_sig_)),
-          original_dex_file_(other.original_dex_file_) {
+          original_dex_file_(other.original_dex_file_),
+          lock_acquired_(other.lock_acquired_) {
       other.driver_ = nullptr;
     }
 
@@ -286,6 +288,7 @@
     bool added_fields_ = false;
     bool added_methods_ = false;
     bool has_virtuals_ = false;
+    bool lock_acquired_ = false;
   };
 
   ArtJvmTiEnv* env_;