Improve handling of daemon threads after runtime shutdown

The main issue comes from the fact that user daemon threads are
allowed to continue running after the runtime has shutdown. They may
still have a JNI env pointer. To prevent crashing if they call into
the env, we replace the function pointers with functions that sleep
forever.

The other issue is that user daemon threads that are blocked in an
ART condition variable may get woken up by another user daemon inside
of Monitor::Notify or by a spurious wakeup (i.e. SIGQUIT). To deal
with this issue, we check the JNI env for shutdown runtime when we
are woken up from a condition variable wait. This check fixes test
132 with --host --gdb --interpreter. Previously this test crashed
since dlclose was somehow causing a spurious futex wakeup.

TODO: Investigate adding a unit test.

Bug: 18577101
Change-Id: I479b38968ee9fbc4ee4b252ee2528787279972cc

diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index 70bd398..82a5f96 100644 (file)
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -855,6 +855,18 @@ void ConditionVariable::WaitHoldingLocks(Thread* self) {
       PLOG(FATAL) << "futex wait failed for " << name_;
     }
   }
+  if (self != nullptr) {
+    JNIEnvExt* const env = self->GetJniEnv();
+    if (UNLIKELY(env != nullptr && env->runtime_deleted)) {
+      CHECK(self->IsDaemon());
+      // If the runtime has been deleted, then we cannot proceed. Just sleep forever. This may
+      // occur for user daemon threads that get a spurious wakeup. This occurs for test 132 with
+      // --host and --gdb.
+      // After we wake up, the runtime may have been shutdown, which means that this condition may
+      // have been deleted. It is not safe to retry the wait.
+      SleepForever();
+    }
+  }
   guard_.ExclusiveLock(self);
   CHECK_GE(num_waiters_, 0);
   num_waiters_--;

diff --git a/runtime/jni_env_ext.cc b/runtime/jni_env_ext.cc
index aa25f67..1ee1611 100644 (file)
--- a/runtime/jni_env_ext.cc
+++ b/runtime/jni_env_ext.cc
@@ -59,6 +59,7 @@ JNIEnvExt::JNIEnvExt(Thread* self_in, JavaVMExt* vm_in)
       local_ref_cookie(IRT_FIRST_SEGMENT),
       locals(kLocalsInitial, kLocalsMax, kLocal, false),
       check_jni(false),
+      runtime_deleted(false),
       critical(0),
       monitors("monitors", kMonitorsInitial, kMonitorsMax) {
   functions = unchecked_functions = GetJniNativeInterface();
@@ -67,6 +68,11 @@ JNIEnvExt::JNIEnvExt(Thread* self_in, JavaVMExt* vm_in)
   }
 }
 
+void JNIEnvExt::SetFunctionsToRuntimeShutdownFunctions() {
+  functions = GetRuntimeShutdownNativeInterface();
+  runtime_deleted = true;
+}
+
 JNIEnvExt::~JNIEnvExt() {
 }
 

diff --git a/runtime/jni_env_ext.h b/runtime/jni_env_ext.h
index 2f8decf..d4accc3 100644 (file)
--- a/runtime/jni_env_ext.h
+++ b/runtime/jni_env_ext.h
@@ -74,6 +74,9 @@ struct JNIEnvExt : public JNIEnv {
   // Frequently-accessed fields cached from JavaVM.
   bool check_jni;
 
+  // If we are a JNI env for a daemon thread with a deleted runtime.
+  bool runtime_deleted;
+
   // How many nested "critical" JNI calls are we in?
   int critical;
 
@@ -95,6 +98,9 @@ struct JNIEnvExt : public JNIEnv {
   // Check that no monitors are held that have been acquired in this JNI "segment."
   void CheckNoHeldMonitors() SHARED_REQUIRES(Locks::mutator_lock_);
 
+  // Set the functions to the runtime shutdown functions.
+  void SetFunctionsToRuntimeShutdownFunctions();
+
  private:
   // The constructor should not be called directly. It may leave the object in an erronuous state,
   // and the result needs to be checked.

diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index cb67ee3..c893a0f 100644 (file)
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -2734,6 +2734,246 @@ const JNINativeInterface* GetJniNativeInterface() {
   return &gJniNativeInterface;
 }
 
+void (*gJniSleepForeverStub[])()  = {
+  nullptr,  // reserved0.
+  nullptr,  // reserved1.
+  nullptr,  // reserved2.
+  nullptr,  // reserved3.
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+  SleepForever,
+};
+
+const JNINativeInterface* GetRuntimeShutdownNativeInterface() {
+  return reinterpret_cast<JNINativeInterface*>(&gJniSleepForeverStub);
+}
+
 void RegisterNativeMethods(JNIEnv* env, const char* jni_class_name, const JNINativeMethod* methods,
                            jint method_count) {
   ScopedLocalRef<jclass> c(env, env->FindClass(jni_class_name));

diff --git a/runtime/jni_internal.h b/runtime/jni_internal.h
index 48b10f5..3429962 100644 (file)
--- a/runtime/jni_internal.h
+++ b/runtime/jni_internal.h
@@ -30,6 +30,7 @@
 namespace art {
 
 const JNINativeInterface* GetJniNativeInterface();
+const JNINativeInterface* GetRuntimeShutdownNativeInterface();
 
 // Similar to RegisterNatives except its passed a descriptor for a class name and failures are
 // fatal.

diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index 77f780f..43ee848 100644 (file)
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -97,8 +97,8 @@ ThreadList::~ThreadList() {
   ATRACE_END();
   // TODO: there's an unaddressed race here where a thread may attach during shutdown, see
   //       Thread::Init.
-  ATRACE_BEGIN("SuspendAllDaemonThreads");
-  SuspendAllDaemonThreads();
+  ATRACE_BEGIN("SuspendAllDaemonThreadsForShutdown");
+  SuspendAllDaemonThreadsForShutdown();
   ATRACE_END();
   ATRACE_END();
 }
@@ -1142,7 +1142,7 @@ void ThreadList::WaitForOtherNonDaemonThreadsToExit() {
   }
 }
 
-void ThreadList::SuspendAllDaemonThreads() {
+void ThreadList::SuspendAllDaemonThreadsForShutdown() {
   Thread* self = Thread::Current();
   MutexLock mu(self, *Locks::thread_list_lock_);
   {  // Tell all the daemons it's time to suspend.
@@ -1154,6 +1154,9 @@ void ThreadList::SuspendAllDaemonThreads() {
       if (thread != self) {
         thread->ModifySuspendCount(self, +1, nullptr, false);
       }
+      // We are shutting down the runtime, set the JNI functions of all the JNIEnvs to be
+      // the sleep forever one.
+      thread->GetJniEnv()->SetFunctionsToRuntimeShutdownFunctions();
     }
   }
   // Give the threads a chance to suspend, complaining if they're slow.

diff --git a/runtime/thread_list.h b/runtime/thread_list.h
index 07ea10d..2e73f6a 100644 (file)
--- a/runtime/thread_list.h
+++ b/runtime/thread_list.h
@@ -164,7 +164,7 @@ class ThreadList {
   void DumpUnattachedThreads(std::ostream& os)
       REQUIRES(!Locks::thread_list_lock_);
 
-  void SuspendAllDaemonThreads()
+  void SuspendAllDaemonThreadsForShutdown()
       REQUIRES(!Locks::thread_list_lock_, !Locks::thread_suspend_count_lock_);
   void WaitForOtherNonDaemonThreadsToExit()
       REQUIRES(!Locks::thread_list_lock_, !Locks::thread_suspend_count_lock_);

diff --git a/runtime/utils.cc b/runtime/utils.cc
index 1e1c7e7..8e9f12b 100644 (file)
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -1871,4 +1871,10 @@ int64_t GetFileSizeBytes(const std::string& filename) {
   return rc == 0 ? stat_buf.st_size : -1;
 }
 
+void SleepForever() {
+  while (true) {
+    usleep(1000000);
+  }
+}
+
 }  // namespace art

diff --git a/runtime/utils.h b/runtime/utils.h
index 5ceb3b5..49406e2 100644 (file)
--- a/runtime/utils.h
+++ b/runtime/utils.h
@@ -373,6 +373,9 @@ T GetRandomNumber(T min, T max) {
 // Return the file size in bytes or -1 if the file does not exists.
 int64_t GetFileSizeBytes(const std::string& filename);
 
+// Sleep forever and never come back.
+NO_RETURN void SleepForever();
+
 }  // namespace art
 
 #endif  // ART_RUNTIME_UTILS_H_