Fix transaction aborting

During compilation, a java.lang.InternalError is used to indicate
that class initialization failed and the enclosing transaction
should be aborted and the changes rolled back. However there is
nothing preventing the code executed from a class initializer from
catching that exception (like catching Throwable and ignore it).
Therefore we may return from the class initializer with no pending
exception, even if the transaction was aborted, and not rollback
the changes properly.

To fix this, we now rely on the new Transaction::aborted_ field to
know whether a transaction aborted. When returning from the class
initializer without pending exception, we now check wether we aborted
the enclosing transaction. If that's the case, we set the status of
the class to kStatusError and throw a new java.lang.InternalError
with the original abort message.

This CL also contains some cleanup:
- Renames Transaction::Abort to Transaction::Rollback which is less
ambiguous and more reflect what is done.
- Moves the code throwing the java.lang.InternalError exception into
the Transaction::ThrowInternalError method so we do not duplicate
code. Now we may abort transaction more than once (because we may
have caught the java.lang.InternalError then execute code causing
new transaction abort), we only keep the first abort message to
throw the exception.
- Updates transaction_test with more cases and more checks.
- Bumps oat version to force recompilation with this fix.

Bug: 19202032
Change-Id: Iedc6969528a68bbdf3123146e990df4dbc57834b

10 files changed, 260 insertions, 71 deletions

diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 7451bd5..2d8c9d4 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -1929,7 +1929,7 @@ static void InitializeClass(const ParallelCompilationManager* manager, size_t cl
                 *file_log << exception->Dump() << "\n";
               }
               soa.Self()->ClearException();
-              transaction.Abort();
+              transaction.Rollback();
               CHECK_EQ(old_status, klass->GetStatus()) << "Previous class status not restored";
             }
           }
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index b66dfeb..e0df6db 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4220,6 +4220,14 @@ bool ClassLinker::InitializeClass(Thread* self, Handle<mirror::Class> klass,
       WrapExceptionInInitializer(klass);
       klass->SetStatus(mirror::Class::kStatusError, self);
       success = false;
+    } else if (Runtime::Current()->IsTransactionAborted()) {
+      // The exception thrown when the transaction aborted has been caught and cleared
+      // so we need to throw it again now.
+      LOG(WARNING) << "Return from class initializer of " << PrettyDescriptor(klass.Get())
+                   << " without exception while transaction was aborted: re-throw it now.";
+      Runtime::Current()->ThrowInternalErrorForAbortedTransaction(self);
+      klass->SetStatus(mirror::Class::kStatusError, self);
+      success = false;
     } else {
       RuntimeStats* global_stats = Runtime::Current()->GetStats();
       RuntimeStats* thread_stats = self->GetStats();
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 2a63456..a29558e 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -546,11 +546,13 @@ static inline void AssignRegister(ShadowFrame* new_shadow_frame, const ShadowFra
 
 void AbortTransaction(Thread* self, const char* fmt, ...) {
   CHECK(Runtime::Current()->IsActiveTransaction());
-  // Throw an exception so we can abort the transaction and undo every change.
+  // Constructs abort message.
   va_list args;
   va_start(args, fmt);
-  self->ThrowNewExceptionV(self->GetCurrentLocationForThrow(), "Ljava/lang/InternalError;", fmt,
-                           args);
+  std::string abort_msg;
+  StringAppendV(&abort_msg, fmt, args);
+  // Throws an exception so we can abort the transaction and rollback every change.
+  Runtime::Current()->AbortTransactionAndThrowInternalError(self, abort_msg);
   va_end(args);
 }
 
diff --git a/runtime/oat.h b/runtime/oat.h
index 3e28606..7faf33b 100644
--- a/runtime/oat.h
+++ b/runtime/oat.h
@@ -32,7 +32,7 @@ class InstructionSetFeatures;
 class PACKED(4) OatHeader {
  public:
   static constexpr uint8_t kOatMagic[] = { 'o', 'a', 't', '\n' };
-  static constexpr uint8_t kOatVersion[] = { '0', '5', '4', '\0' };
+  static constexpr uint8_t kOatVersion[] = { '0', '5', '5', '\0' };
 
   static constexpr const char* kImageLocationKey = "image-location";
   static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline";
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 3acac3a..579ef3f 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1443,6 +1443,31 @@ void Runtime::ExitTransactionMode() {
   preinitialization_transaction_ = nullptr;
 }
 
+
+bool Runtime::IsTransactionAborted() const {
+  if (!IsActiveTransaction()) {
+    return false;
+  } else {
+    DCHECK(IsCompiler());
+    return preinitialization_transaction_->IsAborted();
+  }
+}
+
+void Runtime::AbortTransactionAndThrowInternalError(Thread* self,
+                                                    const std::string& abort_message) {
+  DCHECK(IsCompiler());
+  DCHECK(IsActiveTransaction());
+  preinitialization_transaction_->Abort(abort_message);
+  ThrowInternalErrorForAbortedTransaction(self);
+}
+
+void Runtime::ThrowInternalErrorForAbortedTransaction(Thread* self) {
+  DCHECK(IsCompiler());
+  DCHECK(IsActiveTransaction());
+  DCHECK(IsTransactionAborted());
+  preinitialization_transaction_->ThrowInternalError(self);
+}
+
 void Runtime::RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset,
                                       uint8_t value, bool is_volatile) const {
   DCHECK(IsCompiler());
diff --git a/runtime/runtime.h b/runtime/runtime.h
index c5a8739..118c838 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -455,6 +455,13 @@ class Runtime {
   }
   void EnterTransactionMode(Transaction* transaction);
   void ExitTransactionMode();
+  bool IsTransactionAborted() const;
+
+  void AbortTransactionAndThrowInternalError(Thread* self, const std::string& abort_message)
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+  void ThrowInternalErrorForAbortedTransaction(Thread* self)
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+
   void RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset, uint8_t value,
                                bool is_volatile) const;
   void RecordWriteFieldByte(mirror::Object* obj, MemberOffset field_offset, int8_t value,
diff --git a/runtime/transaction.cc b/runtime/transaction.cc
index 118c1a2..7e2e0a6 100644
--- a/runtime/transaction.cc
+++ b/runtime/transaction.cc
@@ -30,7 +30,8 @@ namespace art {
 // TODO: remove (only used for debugging purpose).
 static constexpr bool kEnableTransactionStats = false;
 
-Transaction::Transaction() : log_lock_("transaction log lock", kTransactionLogLock) {
+Transaction::Transaction()
+  : log_lock_("transaction log lock", kTransactionLogLock), aborted_(false) {
   CHECK(Runtime::Current()->IsCompiler());
 }
 
@@ -57,6 +58,35 @@ Transaction::~Transaction() {
   }
 }
 
+void Transaction::Abort(const std::string& abort_message) {
+  MutexLock mu(Thread::Current(), log_lock_);
+  // We may abort more than once if the java.lang.InternalError thrown at the
+  // time of the abort has been caught during execution of a class initializer.
+  // We just keep the message of the first abort because it will cause the
+  // transaction to be rolled back anyway.
+  if (!aborted_) {
+    aborted_ = true;
+    abort_message_ = abort_message;
+  }
+}
+
+void Transaction::ThrowInternalError(Thread* self) {
+  DCHECK(IsAborted());
+  std::string abort_msg(GetAbortMessage());
+  self->ThrowNewException(self->GetCurrentLocationForThrow(), "Ljava/lang/InternalError;",
+                          abort_msg.c_str());
+}
+
+bool Transaction::IsAborted() {
+  MutexLock mu(Thread::Current(), log_lock_);
+  return aborted_;
+}
+
+const std::string& Transaction::GetAbortMessage() {
+  MutexLock mu(Thread::Current(), log_lock_);
+  return abort_message_;
+}
+
 void Transaction::RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset,
                                           uint8_t value, bool is_volatile) {
   DCHECK(obj != nullptr);
@@ -150,7 +180,7 @@ void Transaction::LogInternedString(const InternStringLog& log) {
   intern_string_logs_.push_front(log);
 }
 
-void Transaction::Abort() {
+void Transaction::Rollback() {
   CHECK(!Runtime::Current()->IsActiveTransaction());
   Thread* self = Thread::Current();
   self->AssertNoPendingException();
diff --git a/runtime/transaction.h b/runtime/transaction.h
index 8c82847..be614f9 100644
--- a/runtime/transaction.h
+++ b/runtime/transaction.h
@@ -42,6 +42,14 @@ class Transaction FINAL {
   Transaction();
   ~Transaction();
 
+  void Abort(const std::string& abort_message)
+      LOCKS_EXCLUDED(log_lock_)
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+  void ThrowInternalError(Thread* self)
+      LOCKS_EXCLUDED(log_lock_)
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+  bool IsAborted() LOCKS_EXCLUDED(log_lock_);
+
   // Record object field changes.
   void RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset, uint8_t value,
                                bool is_volatile)
@@ -85,7 +93,7 @@ class Transaction FINAL {
       LOCKS_EXCLUDED(log_lock_);
 
   // Abort transaction by undoing all recorded changes.
-  void Abort()
+  void Rollback()
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
       LOCKS_EXCLUDED(log_lock_);
 
@@ -206,10 +214,14 @@ class Transaction FINAL {
       EXCLUSIVE_LOCKS_REQUIRED(log_lock_)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
+  const std::string& GetAbortMessage() LOCKS_EXCLUDED(log_lock_);
+
   Mutex log_lock_ ACQUIRED_AFTER(Locks::intern_table_lock_);
   std::map<mirror::Object*, ObjectLog> object_logs_ GUARDED_BY(log_lock_);
   std::map<mirror::Array*, ArrayLog> array_logs_  GUARDED_BY(log_lock_);
   std::list<InternStringLog> intern_string_logs_ GUARDED_BY(log_lock_);
+  bool aborted_ GUARDED_BY(log_lock_);
+  std::string abort_message_ GUARDED_BY(log_lock_);
 
   DISALLOW_COPY_AND_ASSIGN(Transaction);
 };
diff --git a/runtime/transaction_test.cc b/runtime/transaction_test.cc
index 8c4b90d..b80fe22 100644
--- a/runtime/transaction_test.cc
+++ b/runtime/transaction_test.cc
@@ -24,8 +24,68 @@
 
 namespace art {
 
-class TransactionTest : public CommonRuntimeTest {};
-
+class TransactionTest : public CommonRuntimeTest {
+ public:
+  // Tests failing class initialization due to native call with transaction rollback.
+  void testTransactionAbort(const char* tested_class_signature) {
+    ScopedObjectAccess soa(Thread::Current());
+    jobject jclass_loader = LoadDex("Transaction");
+    StackHandleScope<2> hs(soa.Self());
+    Handle<mirror::ClassLoader> class_loader(
+        hs.NewHandle(soa.Decode<mirror::ClassLoader*>(jclass_loader)));
+    ASSERT_TRUE(class_loader.Get() != nullptr);
+
+    // Load and initialize java.lang.ExceptionInInitializerError and java.lang.InternalError
+    // classes so they can be thrown during class initialization if the transaction aborts.
+    MutableHandle<mirror::Class> h_klass(
+        hs.NewHandle(class_linker_->FindSystemClass(soa.Self(),
+                                                    "Ljava/lang/ExceptionInInitializerError;")));
+    ASSERT_TRUE(h_klass.Get() != nullptr);
+    class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+    ASSERT_TRUE(h_klass->IsInitialized());
+
+    h_klass.Assign(class_linker_->FindSystemClass(soa.Self(), "Ljava/lang/InternalError;"));
+    ASSERT_TRUE(h_klass.Get() != nullptr);
+    class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+    ASSERT_TRUE(h_klass->IsInitialized());
+
+    // Load and verify utility class.
+    h_klass.Assign(class_linker_->FindClass(soa.Self(), "LTransaction$AbortHelperClass;",
+                                            class_loader));
+    ASSERT_TRUE(h_klass.Get() != nullptr);
+    class_linker_->VerifyClass(soa.Self(), h_klass);
+    ASSERT_TRUE(h_klass->IsVerified());
+
+    // Load and verify tested class.
+    h_klass.Assign(class_linker_->FindClass(soa.Self(), tested_class_signature, class_loader));
+    ASSERT_TRUE(h_klass.Get() != nullptr);
+    class_linker_->VerifyClass(soa.Self(), h_klass);
+    ASSERT_TRUE(h_klass->IsVerified());
+
+    mirror::Class::Status old_status = h_klass->GetStatus();
+    uint32_t old_lock_word = h_klass->GetLockWord(false).GetValue();
+
+    Transaction transaction;
+    Runtime::Current()->EnterTransactionMode(&transaction);
+    bool success = class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+    Runtime::Current()->ExitTransactionMode();
+    ASSERT_FALSE(success);
+    ASSERT_TRUE(h_klass->IsErroneous());
+    ASSERT_TRUE(soa.Self()->IsExceptionPending());
+    ASSERT_TRUE(transaction.IsAborted());
+
+    // Check class's monitor get back to its original state without rolling back changes.
+    uint32_t new_lock_word = h_klass->GetLockWord(false).GetValue();
+    EXPECT_EQ(old_lock_word, new_lock_word);
+
+    // Check class status is rolled back properly.
+    soa.Self()->ClearException();
+    transaction.Rollback();
+    ASSERT_EQ(old_status, h_klass->GetStatus());
+  }
+};
+
+// Tests object's class is preserved after transaction rollback.
 TEST_F(TransactionTest, Object_class) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<2> hs(soa.Self());
@@ -40,11 +100,12 @@ TEST_F(TransactionTest, Object_class) {
   ASSERT_EQ(h_obj->GetClass(), h_klass.Get());
   Runtime::Current()->ExitTransactionMode();
 
-  // Aborting transaction must not clear the Object::class field.
-  transaction.Abort();
+  // Rolling back transaction's changes must not clear the Object::class field.
+  transaction.Rollback();
   EXPECT_EQ(h_obj->GetClass(), h_klass.Get());
 }
 
+// Tests object's monitor state is preserved after transaction rollback.
 TEST_F(TransactionTest, Object_monitor) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<2> hs(soa.Self());
@@ -66,13 +127,14 @@ TEST_F(TransactionTest, Object_monitor) {
   uint32_t new_lock_word = h_obj->GetLockWord(false).GetValue();
   Runtime::Current()->ExitTransactionMode();
 
-  // Aborting transaction must not clear the Object::class field.
-  transaction.Abort();
+  // Rolling back transaction's changes must not change monitor's state.
+  transaction.Rollback();
   uint32_t aborted_lock_word = h_obj->GetLockWord(false).GetValue();
   EXPECT_NE(old_lock_word, new_lock_word);
   EXPECT_EQ(aborted_lock_word, new_lock_word);
 }
 
+// Tests array's length is preserved after transaction rollback.
 TEST_F(TransactionTest, Array_length) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<2> hs(soa.Self());
@@ -95,11 +157,12 @@ TEST_F(TransactionTest, Array_length) {
   ASSERT_EQ(h_obj->GetClass(), h_klass.Get());
   Runtime::Current()->ExitTransactionMode();
 
-  // Aborting transaction must not clear the Object::class field.
-  transaction.Abort();
+  // Rolling back transaction's changes must not reset array's length.
+  transaction.Rollback();
   EXPECT_EQ(h_obj->GetLength(), kArraySize);
 }
 
+// Tests static fields are reset to their default value after transaction rollback.
 TEST_F(TransactionTest, StaticFieldsTest) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<4> hs(soa.Self());
@@ -110,8 +173,10 @@ TEST_F(TransactionTest, StaticFieldsTest) {
   Handle<mirror::Class> h_klass(
       hs.NewHandle(class_linker_->FindClass(soa.Self(), "LStaticFieldsTest;", class_loader)));
   ASSERT_TRUE(h_klass.Get() != nullptr);
-  class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  bool success = class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  ASSERT_TRUE(success);
   ASSERT_TRUE(h_klass->IsInitialized());
+  ASSERT_FALSE(soa.Self()->IsExceptionPending());
 
   // Lookup fields.
   mirror::ArtField* booleanField = h_klass->FindDeclaredStaticField("booleanField", "Z");
@@ -155,7 +220,7 @@ TEST_F(TransactionTest, StaticFieldsTest) {
   ASSERT_DOUBLE_EQ(doubleField->GetDouble(h_klass.Get()), static_cast<double>(0.0));
 
   mirror::ArtField* objectField = h_klass->FindDeclaredStaticField("objectField",
-                                                                      "Ljava/lang/Object;");
+                                                                   "Ljava/lang/Object;");
   ASSERT_TRUE(objectField != nullptr);
   ASSERT_EQ(objectField->GetTypeAsPrimitiveType(), Primitive::kPrimNot);
   ASSERT_EQ(objectField->GetObject(h_klass.Get()), nullptr);
@@ -168,7 +233,7 @@ TEST_F(TransactionTest, StaticFieldsTest) {
   ASSERT_TRUE(h_obj.Get() != nullptr);
   ASSERT_EQ(h_obj->GetClass(), h_klass.Get());
 
-  // Modify fields inside transaction and abort it.
+  // Modify fields inside transaction then rollback changes.
   Transaction transaction;
   Runtime::Current()->EnterTransactionMode(&transaction);
   booleanField->SetBoolean<true>(h_klass.Get(), true);
@@ -181,7 +246,7 @@ TEST_F(TransactionTest, StaticFieldsTest) {
   doubleField->SetDouble<true>(h_klass.Get(), 1.0);
   objectField->SetObject<true>(h_klass.Get(), h_obj.Get());
   Runtime::Current()->ExitTransactionMode();
-  transaction.Abort();
+  transaction.Rollback();
 
   // Check values have properly been restored to their original (default) value.
   EXPECT_EQ(booleanField->GetBoolean(h_klass.Get()), false);
@@ -195,6 +260,7 @@ TEST_F(TransactionTest, StaticFieldsTest) {
   EXPECT_EQ(objectField->GetObject(h_klass.Get()), nullptr);
 }
 
+// Tests instance fields are reset to their default value after transaction rollback.
 TEST_F(TransactionTest, InstanceFieldsTest) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<5> hs(soa.Self());
@@ -205,8 +271,10 @@ TEST_F(TransactionTest, InstanceFieldsTest) {
   Handle<mirror::Class> h_klass(
       hs.NewHandle(class_linker_->FindClass(soa.Self(), "LInstanceFieldsTest;", class_loader)));
   ASSERT_TRUE(h_klass.Get() != nullptr);
-  class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  bool success = class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  ASSERT_TRUE(success);
   ASSERT_TRUE(h_klass->IsInitialized());
+  ASSERT_FALSE(soa.Self()->IsExceptionPending());
 
   // Allocate an InstanceFieldTest object.
   Handle<mirror::Object> h_instance(hs.NewHandle(h_klass->AllocObject(soa.Self())));
@@ -267,7 +335,7 @@ TEST_F(TransactionTest, InstanceFieldsTest) {
   ASSERT_TRUE(h_obj.Get() != nullptr);
   ASSERT_EQ(h_obj->GetClass(), h_klass.Get());
 
-  // Modify fields inside transaction and abort it.
+  // Modify fields inside transaction then rollback changes.
   Transaction transaction;
   Runtime::Current()->EnterTransactionMode(&transaction);
   booleanField->SetBoolean<true>(h_instance.Get(), true);
@@ -280,7 +348,7 @@ TEST_F(TransactionTest, InstanceFieldsTest) {
   doubleField->SetDouble<true>(h_instance.Get(), 1.0);
   objectField->SetObject<true>(h_instance.Get(), h_obj.Get());
   Runtime::Current()->ExitTransactionMode();
-  transaction.Abort();
+  transaction.Rollback();
 
   // Check values have properly been restored to their original (default) value.
   EXPECT_EQ(booleanField->GetBoolean(h_instance.Get()), false);
@@ -294,7 +362,7 @@ TEST_F(TransactionTest, InstanceFieldsTest) {
   EXPECT_EQ(objectField->GetObject(h_instance.Get()), nullptr);
 }
 
-
+// Tests static array fields are reset to their default value after transaction rollback.
 TEST_F(TransactionTest, StaticArrayFieldsTest) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<4> hs(soa.Self());
@@ -305,8 +373,10 @@ TEST_F(TransactionTest, StaticArrayFieldsTest) {
   Handle<mirror::Class> h_klass(
       hs.NewHandle(class_linker_->FindClass(soa.Self(), "LStaticArrayFieldsTest;", class_loader)));
   ASSERT_TRUE(h_klass.Get() != nullptr);
-  class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  bool success = class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  ASSERT_TRUE(success);
   ASSERT_TRUE(h_klass->IsInitialized());
+  ASSERT_FALSE(soa.Self()->IsExceptionPending());
 
   // Lookup fields.
   mirror::ArtField* booleanArrayField = h_klass->FindDeclaredStaticField("booleanArrayField", "[Z");
@@ -382,7 +452,7 @@ TEST_F(TransactionTest, StaticArrayFieldsTest) {
   ASSERT_TRUE(h_obj.Get() != nullptr);
   ASSERT_EQ(h_obj->GetClass(), h_klass.Get());
 
-  // Modify fields inside transaction and abort it.
+  // Modify fields inside transaction then rollback changes.
   Transaction transaction;
   Runtime::Current()->EnterTransactionMode(&transaction);
   booleanArray->SetWithoutChecks<true>(0, true);
@@ -395,7 +465,7 @@ TEST_F(TransactionTest, StaticArrayFieldsTest) {
   doubleArray->SetWithoutChecks<true>(0, 1.0);
   objectArray->SetWithoutChecks<true>(0, h_obj.Get());
   Runtime::Current()->ExitTransactionMode();
-  transaction.Abort();
+  transaction.Rollback();
 
   // Check values have properly been restored to their original (default) value.
   EXPECT_EQ(booleanArray->GetWithoutChecks(0), false);
@@ -409,6 +479,7 @@ TEST_F(TransactionTest, StaticArrayFieldsTest) {
   EXPECT_EQ(objectArray->GetWithoutChecks(0), nullptr);
 }
 
+// Tests successful class initialization without class initializer.
 TEST_F(TransactionTest, EmptyClass) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<2> hs(soa.Self());
@@ -417,18 +488,22 @@ TEST_F(TransactionTest, EmptyClass) {
   ASSERT_TRUE(class_loader.Get() != nullptr);
 
   Handle<mirror::Class> h_klass(
-      hs.NewHandle(class_linker_->FindClass(soa.Self(), "LTransaction$EmptyStatic;", class_loader)));
+      hs.NewHandle(class_linker_->FindClass(soa.Self(), "LTransaction$EmptyStatic;",
+                                            class_loader)));
   ASSERT_TRUE(h_klass.Get() != nullptr);
   class_linker_->VerifyClass(soa.Self(), h_klass);
   ASSERT_TRUE(h_klass->IsVerified());
 
   Transaction transaction;
   Runtime::Current()->EnterTransactionMode(&transaction);
-  class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  bool success = class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
   Runtime::Current()->ExitTransactionMode();
+  ASSERT_TRUE(success);
+  ASSERT_TRUE(h_klass->IsInitialized());
   ASSERT_FALSE(soa.Self()->IsExceptionPending());
 }
 
+// Tests successful class initialization with class initializer.
 TEST_F(TransactionTest, StaticFieldClass) {
   ScopedObjectAccess soa(Thread::Current());
   StackHandleScope<2> hs(soa.Self());
@@ -445,51 +520,37 @@ TEST_F(TransactionTest, StaticFieldClass) {
 
   Transaction transaction;
   Runtime::Current()->EnterTransactionMode(&transaction);
-  class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
+  bool success = class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
   Runtime::Current()->ExitTransactionMode();
+  ASSERT_TRUE(success);
+  ASSERT_TRUE(h_klass->IsInitialized());
   ASSERT_FALSE(soa.Self()->IsExceptionPending());
 }
 
-TEST_F(TransactionTest, BlacklistedClass) {
-  ScopedObjectAccess soa(Thread::Current());
-  jobject jclass_loader = LoadDex("Transaction");
-  StackHandleScope<2> hs(soa.Self());
-  Handle<mirror::ClassLoader> class_loader(
-      hs.NewHandle(soa.Decode<mirror::ClassLoader*>(jclass_loader)));
-  ASSERT_TRUE(class_loader.Get() != nullptr);
-
-  // Load and verify java.lang.ExceptionInInitializerError and java.lang.InternalError which will
-  // be thrown by class initialization due to native call.
-  MutableHandle<mirror::Class> h_klass(
-      hs.NewHandle(class_linker_->FindSystemClass(soa.Self(),
-                                                  "Ljava/lang/ExceptionInInitializerError;")));
-  ASSERT_TRUE(h_klass.Get() != nullptr);
-  class_linker_->VerifyClass(soa.Self(), h_klass);
-  ASSERT_TRUE(h_klass->IsVerified());
-  h_klass.Assign(class_linker_->FindSystemClass(soa.Self(), "Ljava/lang/InternalError;"));
-  ASSERT_TRUE(h_klass.Get() != nullptr);
-  class_linker_->VerifyClass(soa.Self(), h_klass);
-  ASSERT_TRUE(h_klass->IsVerified());
-
-  // Load and verify Transaction$NativeSupport used in class initialization.
-  h_klass.Assign(class_linker_->FindClass(soa.Self(), "LTransaction$NativeSupport;",
-                                             class_loader));
-  ASSERT_TRUE(h_klass.Get() != nullptr);
-  class_linker_->VerifyClass(soa.Self(), h_klass);
-  ASSERT_TRUE(h_klass->IsVerified());
+// Tests failing class initialization due to native call.
+TEST_F(TransactionTest, NativeCallAbortClass) {
+  testTransactionAbort("LTransaction$NativeCallAbortClass;");
+}
 
-  h_klass.Assign(class_linker_->FindClass(soa.Self(), "LTransaction$BlacklistedClass;",
-                                             class_loader));
-  ASSERT_TRUE(h_klass.Get() != nullptr);
-  class_linker_->VerifyClass(soa.Self(), h_klass);
-  ASSERT_TRUE(h_klass->IsVerified());
+// Tests failing class initialization due to native call in a "synchronized" statement
+// (which must catch any exception, do the monitor-exit then re-throw the caught exception).
+TEST_F(TransactionTest, SynchronizedNativeCallAbortClass) {
+  testTransactionAbort("LTransaction$SynchronizedNativeCallAbortClass;");
+}
 
-  Transaction transaction;
-  Runtime::Current()->EnterTransactionMode(&transaction);
-  class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
-  Runtime::Current()->ExitTransactionMode();
-  ASSERT_TRUE(soa.Self()->IsExceptionPending());
+// Tests failing class initialization due to native call, even if an "all" catch handler
+// catches the exception thrown when aborting the transaction.
+TEST_F(TransactionTest, CatchNativeCallAbortClass) {
+  testTransactionAbort("LTransaction$CatchNativeCallAbortClass;");
 }
 
+// Tests failing class initialization with multiple transaction aborts.
+TEST_F(TransactionTest, MultipleNativeCallAbortClass) {
+  testTransactionAbort("LTransaction$MultipleNativeCallAbortClass;");
+}
 
+// Tests failing class initialization due to allocating instance of finalizable class.
+TEST_F(TransactionTest, FinalizableAbortClass) {
+  testTransactionAbort("LTransaction$FinalizableAbortClass;");
+}
 }  // namespace art
diff --git a/test/Transaction/Transaction.java b/test/Transaction/Transaction.java
index 9ca7fbf..00e1fbb 100644
--- a/test/Transaction/Transaction.java
+++ b/test/Transaction/Transaction.java
@@ -25,13 +25,57 @@ public class Transaction {
       }
     }
 
-    static class BlacklistedClass {
+    static class FinalizableAbortClass {
+      public static AbortHelperClass finalizableObject;
       static {
-        NativeSupport.native_call();
+        finalizableObject = new AbortHelperClass();
       }
     }
 
-    static class NativeSupport {
-      public static native void native_call();
+    static class NativeCallAbortClass {
+      static {
+        AbortHelperClass.nativeMethod();
+      }
+    }
+
+    static class SynchronizedNativeCallAbortClass {
+      static {
+        synchronized (SynchronizedNativeCallAbortClass.class) {
+          AbortHelperClass.nativeMethod();
+        }
+      }
+    }
+
+    static class CatchNativeCallAbortClass {
+      static {
+        try {
+          AbortHelperClass.nativeMethod();
+        } catch (Throwable e) {
+          // ignore exception.
+        }
+      }
+    }
+
+    static class MultipleNativeCallAbortClass {
+      static {
+        // Call native method but catch the transaction exception.
+        try {
+          AbortHelperClass.nativeMethod();
+        } catch (Throwable e) {
+          // ignore exception.
+        }
+
+        // Call another native method.
+        AbortHelperClass.nativeMethod2();
+      }
+    }
+
+    // Helper class to abort transaction: finalizable class with natve methods.
+    static class AbortHelperClass {
+      public void finalize() throws Throwable {
+        super.finalize();
+      }
+      public static native void nativeMethod();
+      public static native void nativeMethod2();
     }
 }