Revert "Tidy gAborting."

Creates infinite loop: b/18674776.

This reverts commit 015b137efb434528173779bc3ec8d72494456254.

Change-Id: I67fe310d2e95ee2ec37bec842be06fb1123b6f4e

12 files changed, 64 insertions, 52 deletions

diff --git a/runtime/base/logging.cc b/runtime/base/logging.cc
index bdc4cf6..b781d60 100644
--- a/runtime/base/logging.cc
+++ b/runtime/base/logging.cc
@@ -35,6 +35,8 @@ namespace art {
 
 LogVerbosity gLogVerbosity;
 
+unsigned int gAborting = 0;
+
 static LogSeverity gMinimumLogSeverity = INFO;
 static std::unique_ptr<std::string> gCmdLine;
 static std::unique_ptr<std::string> gProgramInvocationName;
diff --git a/runtime/base/logging.h b/runtime/base/logging.h
index a9cc99b..ae83e33 100644
--- a/runtime/base/logging.h
+++ b/runtime/base/logging.h
@@ -55,6 +55,11 @@ struct LogVerbosity {
 // Global log verbosity setting, initialized by InitLogging.
 extern LogVerbosity gLogVerbosity;
 
+// 0 if not abort, non-zero if an abort is in progress. Used on fatal exit to prevents recursive
+// aborts. Global declaration allows us to disable some error checking to ensure fatal shutdown
+// makes forward progress.
+extern unsigned int gAborting;
+
 // Configure logging based on ANDROID_LOG_TAGS environment variable.
 // We need to parse a string that looks like
 //
diff --git a/runtime/base/mutex-inl.h b/runtime/base/mutex-inl.h
index 0206341..cb69817 100644
--- a/runtime/base/mutex-inl.h
+++ b/runtime/base/mutex-inl.h
@@ -97,7 +97,9 @@ inline void BaseMutex::RegisterAsLocked(Thread* self) {
         }
       }
     }
-    CHECK(!bad_mutexes_held);
+    if (gAborting == 0) {  // Avoid recursive aborts.
+      CHECK(!bad_mutexes_held);
+    }
   }
   // Don't record monitors as they are outside the scope of analysis. They may be inspected off of
   // the monitor list.
@@ -112,7 +114,7 @@ inline void BaseMutex::RegisterAsUnlocked(Thread* self) {
     return;
   }
   if (level_ != kMonitorLock) {
-    if (kDebugLocking) {
+    if (kDebugLocking && gAborting == 0) {  // Avoid recursive aborts.
       CHECK(self->GetHeldMutex(level_) == this) << "Unlocking on unacquired mutex: " << name_;
     }
     self->SetHeldMutex(level_, NULL);
@@ -176,7 +178,7 @@ inline bool Mutex::IsExclusiveHeld(const Thread* self) const {
   bool result = (GetExclusiveOwnerTid() == SafeGetTid(self));
   if (kDebugLocking) {
     // Sanity debug check that if we think it is locked we have it in our held mutexes.
-    if (result && self != NULL && level_ != kMonitorLock) {
+    if (result && self != NULL && level_ != kMonitorLock && !gAborting) {
       CHECK_EQ(self->GetHeldMutex(level_), this);
     }
   }
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index 4957988..aa2aefc 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -209,7 +209,9 @@ void BaseMutex::CheckSafeToWait(Thread* self) {
         }
       }
     }
-    CHECK(!bad_mutexes_held);
+    if (gAborting == 0) {  // Avoid recursive aborts.
+      CHECK(!bad_mutexes_held);
+    }
   }
 }
 
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index 41b5f12..9c93cc6 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -220,7 +220,7 @@ class LOCKABLE Mutex : public BaseMutex {
 
   // Assert that the Mutex is exclusively held by the current thread.
   void AssertExclusiveHeld(const Thread* self) {
-    if (kDebugLocking) {
+    if (kDebugLocking && (gAborting == 0)) {
       CHECK(IsExclusiveHeld(self)) << *this;
     }
   }
@@ -228,7 +228,7 @@ class LOCKABLE Mutex : public BaseMutex {
 
   // Assert that the Mutex is not held by the current thread.
   void AssertNotHeldExclusive(const Thread* self) {
-    if (kDebugLocking) {
+    if (kDebugLocking && (gAborting == 0)) {
       CHECK(!IsExclusiveHeld(self)) << *this;
     }
   }
@@ -318,7 +318,7 @@ class LOCKABLE ReaderWriterMutex : public BaseMutex {
 
   // Assert the current thread has exclusive access to the ReaderWriterMutex.
   void AssertExclusiveHeld(const Thread* self) {
-    if (kDebugLocking) {
+    if (kDebugLocking && (gAborting == 0)) {
       CHECK(IsExclusiveHeld(self)) << *this;
     }
   }
@@ -326,7 +326,7 @@ class LOCKABLE ReaderWriterMutex : public BaseMutex {
 
   // Assert the current thread doesn't have exclusive access to the ReaderWriterMutex.
   void AssertNotExclusiveHeld(const Thread* self) {
-    if (kDebugLocking) {
+    if (kDebugLocking && (gAborting == 0)) {
       CHECK(!IsExclusiveHeld(self)) << *this;
     }
   }
@@ -337,7 +337,7 @@ class LOCKABLE ReaderWriterMutex : public BaseMutex {
 
   // Assert the current thread has shared access to the ReaderWriterMutex.
   void AssertSharedHeld(const Thread* self) {
-    if (kDebugLocking) {
+    if (kDebugLocking && (gAborting == 0)) {
       // TODO: we can only assert this well when self != NULL.
       CHECK(IsSharedHeld(self) || self == NULL) << *this;
     }
@@ -347,7 +347,7 @@ class LOCKABLE ReaderWriterMutex : public BaseMutex {
   // Assert the current thread doesn't hold this ReaderWriterMutex either in shared or exclusive
   // mode.
   void AssertNotHeld(const Thread* self) {
-    if (kDebugLocking) {
+    if (kDebugLocking && (gAborting == 0)) {
       CHECK(!IsSharedHeld(self)) << *this;
     }
   }
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index e792031..078e7d2 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -126,8 +126,6 @@ namespace art {
 static constexpr bool kEnableJavaStackTraceHandler = false;
 Runtime* Runtime::instance_ = nullptr;
 
-volatile unsigned int gAborting = 0;
-
 Runtime::Runtime()
     : instruction_set_(kNone),
       compiler_callbacks_(nullptr),
@@ -238,8 +236,13 @@ Runtime::~Runtime() {
 
 struct AbortState {
   void Dump(std::ostream& os) const {
+    if (gAborting > 1) {
+      os << "Runtime aborting --- recursively, so no thread-specific detail!\n";
+      return;
+    }
+    gAborting++;
     os << "Runtime aborting...\n";
-    if (Runtime::Current() == nullptr) {
+    if (Runtime::Current() == NULL) {
       os << "(Runtime does not yet exist!)\n";
       return;
     }
@@ -297,18 +300,13 @@ struct AbortState {
 
 void Runtime::Abort() {
   gAborting++;  // set before taking any locks
-  if (gAborting > 1) {
-    LogMessage::LogLine(__FILE__, __LINE__, INTERNAL_FATAL,
-                        "Runtime aborting --- recursively, so no thread-specific detail!\n");
-    return;
-  }
 
   // Ensure that we don't have multiple threads trying to abort at once,
   // which would result in significantly worse diagnostics.
   MutexLock mu(Thread::Current(), *Locks::abort_lock_);
 
   // Get any pending output out of the way.
-  fflush(nullptr);
+  fflush(NULL);
 
   // Many people have difficulty distinguish aborts from crashes,
   // so be explicit.
@@ -316,7 +314,7 @@ void Runtime::Abort() {
   LOG(INTERNAL_FATAL) << Dumpable<AbortState>(state);
 
   // Call the abort hook if we have one.
-  if (Runtime::Current() != nullptr && Runtime::Current()->abort_ != nullptr) {
+  if (Runtime::Current() != NULL && Runtime::Current()->abort_ != NULL) {
     LOG(INTERNAL_FATAL) << "Calling abort hook...";
     Runtime::Current()->abort_();
     // notreached
@@ -344,7 +342,7 @@ void Runtime::PreZygoteFork() {
 }
 
 void Runtime::CallExitHook(jint status) {
-  if (exit_ != nullptr) {
+  if (exit_ != NULL) {
     ScopedThreadStateChange tsc(Thread::Current(), kNative);
     exit_(status);
     LOG(WARNING) << "Exit hook returned instead of exiting!";
@@ -359,14 +357,14 @@ void Runtime::SweepSystemWeaks(IsMarkedCallback* visitor, void* arg) {
 
 bool Runtime::Create(const RuntimeOptions& options, bool ignore_unrecognized) {
   // TODO: acquire a static mutex on Runtime to avoid racing.
-  if (Runtime::instance_ != nullptr) {
+  if (Runtime::instance_ != NULL) {
     return false;
   }
-  InitLogging(nullptr);  // Calls Locks::Init() as a side effect.
+  InitLogging(NULL);  // Calls Locks::Init() as a side effect.
   instance_ = new Runtime;
   if (!instance_->Init(options, ignore_unrecognized)) {
     delete instance_;
-    instance_ = nullptr;
+    instance_ = NULL;
     return false;
   }
   return true;
@@ -374,7 +372,7 @@ bool Runtime::Create(const RuntimeOptions& options, bool ignore_unrecognized) {
 
 static jobject CreateSystemClassLoader() {
   if (Runtime::Current()->UseCompileTimeClassPath()) {
-    return nullptr;
+    return NULL;
   }
 
   ScopedObjectAccess soa(Thread::Current());
@@ -387,7 +385,7 @@ static jobject CreateSystemClassLoader() {
 
   mirror::ArtMethod* getSystemClassLoader =
       class_loader_class->FindDirectMethod("getSystemClassLoader", "()Ljava/lang/ClassLoader;");
-  CHECK(getSystemClassLoader != nullptr);
+  CHECK(getSystemClassLoader != NULL);
 
   JValue result = InvokeWithJValues(soa, nullptr, soa.EncodeMethod(getSystemClassLoader), nullptr);
   JNIEnv* env = soa.Self()->GetJniEnv();
@@ -403,7 +401,7 @@ static jobject CreateSystemClassLoader() {
 
   mirror::ArtField* contextClassLoader =
       thread_class->FindDeclaredInstanceField("contextClassLoader", "Ljava/lang/ClassLoader;");
-  CHECK(contextClassLoader != nullptr);
+  CHECK(contextClassLoader != NULL);
 
   // We can't run in a transaction yet.
   contextClassLoader->SetObject<false>(soa.Self()->GetPeer(),
@@ -529,7 +527,7 @@ bool Runtime::InitZygote() {
 
   // Mark rootfs as being a slave so that changes from default
   // namespace only flow into our children.
-  if (mount("rootfs", "/", nullptr, (MS_SLAVE | MS_REC), nullptr) == -1) {
+  if (mount("rootfs", "/", NULL, (MS_SLAVE | MS_REC), NULL) == -1) {
     PLOG(WARNING) << "Failed to mount() rootfs as MS_SLAVE";
     return false;
   }
@@ -538,7 +536,7 @@ bool Runtime::InitZygote() {
   // bind mount storage into their respective private namespaces, which
   // are isolated from each other.
   const char* target_base = getenv("EMULATED_STORAGE_TARGET");
-  if (target_base != nullptr) {
+  if (target_base != NULL) {
     if (mount("tmpfs", target_base, "tmpfs", MS_NOSUID | MS_NODEV,
               "uid=0,gid=1028,mode=0751") == -1) {
       LOG(WARNING) << "Failed to mount tmpfs to " << target_base;
@@ -895,14 +893,14 @@ bool Runtime::Init(const RuntimeOptions& raw_options, bool ignore_unrecognized)
   self->ThrowNewException(ThrowLocation(), "Ljava/lang/OutOfMemoryError;",
                           "OutOfMemoryError thrown while trying to throw OutOfMemoryError; "
                           "no stack trace available");
-  pre_allocated_OutOfMemoryError_ = GcRoot<mirror::Throwable>(self->GetException(nullptr));
+  pre_allocated_OutOfMemoryError_ = GcRoot<mirror::Throwable>(self->GetException(NULL));
   self->ClearException();
 
   // Pre-allocate a NoClassDefFoundError for the common case of failing to find a system class
   // ahead of checking the application's class loader.
   self->ThrowNewException(ThrowLocation(), "Ljava/lang/NoClassDefFoundError;",
                           "Class not found using the boot class loader; no stack trace available");
-  pre_allocated_NoClassDefFoundError_ = GcRoot<mirror::Throwable>(self->GetException(nullptr));
+  pre_allocated_NoClassDefFoundError_ = GcRoot<mirror::Throwable>(self->GetException(NULL));
   self->ClearException();
 
   // Look for a native bridge.
@@ -978,26 +976,26 @@ void Runtime::InitThreadGroups(Thread* self) {
       env->NewGlobalRef(env->GetStaticObjectField(
           WellKnownClasses::java_lang_ThreadGroup,
           WellKnownClasses::java_lang_ThreadGroup_mainThreadGroup));
-  CHECK(main_thread_group_ != nullptr || IsCompiler());
+  CHECK(main_thread_group_ != NULL || IsCompiler());
   system_thread_group_ =
       env->NewGlobalRef(env->GetStaticObjectField(
           WellKnownClasses::java_lang_ThreadGroup,
           WellKnownClasses::java_lang_ThreadGroup_systemThreadGroup));
-  CHECK(system_thread_group_ != nullptr || IsCompiler());
+  CHECK(system_thread_group_ != NULL || IsCompiler());
 }
 
 jobject Runtime::GetMainThreadGroup() const {
-  CHECK(main_thread_group_ != nullptr || IsCompiler());
+  CHECK(main_thread_group_ != NULL || IsCompiler());
   return main_thread_group_;
 }
 
 jobject Runtime::GetSystemThreadGroup() const {
-  CHECK(system_thread_group_ != nullptr || IsCompiler());
+  CHECK(system_thread_group_ != NULL || IsCompiler());
   return system_thread_group_;
 }
 
 jobject Runtime::GetSystemClassLoader() const {
-  CHECK(system_class_loader_ != nullptr || IsCompiler());
+  CHECK(system_class_loader_ != NULL || IsCompiler());
   return system_class_loader_;
 }
 
@@ -1123,12 +1121,12 @@ void Runtime::BlockSignals() {
 
 bool Runtime::AttachCurrentThread(const char* thread_name, bool as_daemon, jobject thread_group,
                                   bool create_peer) {
-  return Thread::Attach(thread_name, as_daemon, thread_group, create_peer) != nullptr;
+  return Thread::Attach(thread_name, as_daemon, thread_group, create_peer) != NULL;
 }
 
 void Runtime::DetachCurrentThread() {
   Thread* self = Thread::Current();
-  if (self == nullptr) {
+  if (self == NULL) {
     LOG(FATAL) << "attempting to detach thread that is not attached";
   }
   if (self->HasManagedStack()) {
@@ -1353,7 +1351,7 @@ void Runtime::SetCalleeSaveMethod(mirror::ArtMethod* method, CalleeSaveType type
 }
 
 const std::vector<const DexFile*>& Runtime::GetCompileTimeClassPath(jobject class_loader) {
-  if (class_loader == nullptr) {
+  if (class_loader == NULL) {
     return GetClassLinker()->GetBootClassPath();
   }
   CHECK(UseCompileTimeClassPath());
diff --git a/runtime/runtime.h b/runtime/runtime.h
index e334764..39fd910 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -71,11 +71,6 @@ class ThreadList;
 class Trace;
 class Transaction;
 
-// 0 if not abort, non-zero if an abort is in progress. Used on fatal exit to prevents recursive
-// aborts. Global declaration allows us to disable some error checking to ensure fatal shutdown
-// makes forward progress.
-extern volatile unsigned int gAborting;
-
 typedef std::vector<std::pair<std::string, const void*>> RuntimeOptions;
 
 // Not all combinations of flags are valid. You may not visit all roots as well as the new roots
@@ -180,9 +175,9 @@ class Runtime {
     return instance_;
   }
 
-  // Aborts semi-cleanly. Used in the implementation of LOG(FATAL), which most callers should
-  // prefer. Not [[noreturn]] due to returning early in the case of recursive aborts.
-  static void Abort() LOCKS_EXCLUDED(Locks::abort_lock_);
+  // Aborts semi-cleanly. Used in the implementation of LOG(FATAL), which most
+  // callers should prefer.
+  [[noreturn]] static void Abort() LOCKS_EXCLUDED(Locks::abort_lock_);
 
   // Returns the "main" ThreadGroup, used when attaching user threads.
   jobject GetMainThreadGroup() const;
diff --git a/runtime/runtime_android.cc b/runtime/runtime_android.cc
index 33641ed..33600dd 100644
--- a/runtime/runtime_android.cc
+++ b/runtime/runtime_android.cc
@@ -38,6 +38,7 @@ void HandleUnexpectedSignal(int signal_number, siginfo_t* info, void* raw_contex
     _exit(1);
   }
   handling_unexpected_signal = true;
+  gAborting++;  // set before taking any locks
   MutexLock mu(Thread::Current(), *Locks::unexpected_signal_lock_);
 
   Runtime* runtime = Runtime::Current();
diff --git a/runtime/runtime_linux.cc b/runtime/runtime_linux.cc
index 9273091..1de035c 100644
--- a/runtime/runtime_linux.cc
+++ b/runtime/runtime_linux.cc
@@ -284,6 +284,7 @@ void HandleUnexpectedSignal(int signal_number, siginfo_t* info, void* raw_contex
   }
   handlingUnexpectedSignal = true;
 
+  gAborting++;  // set before taking any locks
   MutexLock mu(Thread::Current(), *Locks::unexpected_signal_lock_);
 
   bool has_address = (signal_number == SIGILL || signal_number == SIGBUS ||
diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h
index 49b7be9..7aed8b0 100644
--- a/runtime/thread-inl.h
+++ b/runtime/thread-inl.h
@@ -83,7 +83,9 @@ inline ThreadState Thread::SetState(ThreadState new_state) {
 
 inline void Thread::AssertThreadSuspensionIsAllowable(bool check_locks) const {
   if (kIsDebugBuild) {
-    CHECK_EQ(0u, tls32_.no_thread_suspension) << tlsPtr_.last_no_thread_suspension_cause;
+    if (gAborting == 0) {
+      CHECK_EQ(0u, tls32_.no_thread_suspension) << tlsPtr_.last_no_thread_suspension_cause;
+    }
     if (check_locks) {
       bool bad_mutexes_held = false;
       for (int i = kLockLevelCount - 1; i >= 0; --i) {
@@ -97,7 +99,9 @@ inline void Thread::AssertThreadSuspensionIsAllowable(bool check_locks) const {
           }
         }
       }
-      CHECK(!bad_mutexes_held);
+      if (gAborting == 0) {
+        CHECK(!bad_mutexes_held);
+      }
     }
   }
 }
diff --git a/runtime/thread.cc b/runtime/thread.cc
index a6e5b0c..f7c7106 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -742,7 +742,7 @@ void Thread::DumpState(std::ostream& os, const Thread* thread, pid_t tid) {
 
   // Don't do this if we are aborting since the GC may have all the threads suspended. This will
   // cause ScopedObjectAccessUnchecked to deadlock.
-  if (self != nullptr && thread != nullptr && thread->tlsPtr_.opeer != nullptr) {
+  if (gAborting == 0 && self != nullptr && thread != nullptr && thread->tlsPtr_.opeer != nullptr) {
     ScopedObjectAccessUnchecked soa(self);
     priority = soa.DecodeField(WellKnownClasses::java_lang_Thread_priority)
         ->GetInt(thread->tlsPtr_.opeer);
diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index 71325a5..beafcda 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -168,7 +168,9 @@ class DumpCheckpoint FINAL : public Closure {
     const uint32_t kWaitTimeoutMs = 10000;
     bool timed_out = barrier_.Increment(self, threads_running_checkpoint, kWaitTimeoutMs);
     if (timed_out) {
-      LOG(kIsDebugBuild ? FATAL : ERROR) << "Unexpected time out during dump checkpoint.";
+      // Avoid a recursive abort.
+      LOG((kIsDebugBuild && (gAborting == 0)) ? FATAL : ERROR)
+          << "Unexpected time out during dump checkpoint.";
     }
   }
 
@@ -241,7 +243,7 @@ size_t ThreadList::RunCheckpoint(Closure* checkpoint_function) {
   Locks::mutator_lock_->AssertNotExclusiveHeld(self);
   Locks::thread_list_lock_->AssertNotHeld(self);
   Locks::thread_suspend_count_lock_->AssertNotHeld(self);
-  if (kDebugLocking) {
+  if (kDebugLocking && gAborting == 0) {
     CHECK_NE(self->GetState(), kRunnable);
   }