diff options
author | Mathieu Chartier <mathieuc@google.com> | 2016-01-07 15:14:19 -0800 |
---|---|---|
committer | Mathieu Chartier <mathieuc@google.com> | 2016-01-08 15:46:52 -0800 |
commit | 4d87df607a0b86cdf4b2c04f61d72a60d8975ce0 (patch) | |
tree | 922e882c15633e6c700091c99864868d69822d7d | |
parent | 97f2ca08c3d9a2b1694419aea07cd64f477c0af2 (diff) | |
download | art-4d87df607a0b86cdf4b2c04f61d72a60d8975ce0.tar.gz art-4d87df607a0b86cdf4b2c04f61d72a60d8975ce0.tar.bz2 art-4d87df607a0b86cdf4b2c04f61d72a60d8975ce0.zip |
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
-rw-r--r-- | runtime/base/mutex.cc | 12 | ||||
-rw-r--r-- | runtime/jni_env_ext.cc | 6 | ||||
-rw-r--r-- | runtime/jni_env_ext.h | 6 | ||||
-rw-r--r-- | runtime/jni_internal.cc | 240 | ||||
-rw-r--r-- | runtime/jni_internal.h | 1 | ||||
-rw-r--r-- | runtime/thread_list.cc | 9 | ||||
-rw-r--r-- | runtime/thread_list.h | 2 | ||||
-rw-r--r-- | runtime/utils.cc | 6 | ||||
-rw-r--r-- | runtime/utils.h | 3 |
9 files changed, 281 insertions, 4 deletions
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc index 70bd398415..82a5f9611c 100644 --- 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 aa25f67bab..1ee1611ef7 100644 --- 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 2f8decf98f..d4accc342b 100644 --- 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 cb67ee3b39..c893a0fc9d 100644 --- 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 48b10f5825..3429962d3f 100644 --- 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 77f780fe11..43ee84839b 100644 --- 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 07ea10dbea..2e73f6af7f 100644 --- 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 1e1c7e7098..8e9f12b7a0 100644 --- 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 5ceb3b5cd4..49406e20c9 100644 --- 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_ |