diff options
-rw-r--r-- | runtime/base/mutex-inl.h | 4 | ||||
-rw-r--r-- | runtime/base/mutex.cc | 68 | ||||
-rw-r--r-- | runtime/base/mutex.h | 12 | ||||
-rw-r--r-- | runtime/runtime.cc | 9 | ||||
-rw-r--r-- | runtime/thread-inl.h | 1 |
5 files changed, 54 insertions, 40 deletions
diff --git a/runtime/base/mutex-inl.h b/runtime/base/mutex-inl.h index 1c320243dc..92b7c6537c 100644 --- a/runtime/base/mutex-inl.h +++ b/runtime/base/mutex-inl.h @@ -23,7 +23,6 @@ #include "base/stringprintf.h" #include "base/value_object.h" -#include "runtime.h" #include "thread.h" #include "utils.h" @@ -59,8 +58,7 @@ static inline void CheckUnattachedThread(LockLevel level) NO_THREAD_SAFETY_ANALY // on a thread. Lock checking is disabled to avoid deadlock when checking shutdown lock. // TODO: tighten this check. if (kDebugLocking) { - Runtime* runtime = Runtime::Current(); - CHECK(runtime == nullptr || !runtime->IsStarted() || runtime->IsShuttingDownLocked() || + CHECK(!Locks::IsSafeToCallAbortRacy() || // Used during thread creation to avoid races with runtime shutdown. Thread::Current not // yet established. level == kRuntimeShutdownLock || diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc index e77e6d7170..bde03277b7 100644 --- a/runtime/base/mutex.cc +++ b/runtime/base/mutex.cc @@ -25,12 +25,13 @@ #include "base/systrace.h" #include "base/value_object.h" #include "mutex-inl.h" -#include "runtime.h" #include "scoped_thread_state_change-inl.h" #include "thread-inl.h" namespace art { +static Atomic<Locks::ClientCallback*> safe_to_call_abort_callback(nullptr); + Mutex* Locks::abort_lock_ = nullptr; Mutex* Locks::alloc_tracker_lock_ = nullptr; Mutex* Locks::allocated_monitor_ids_lock_ = nullptr; @@ -320,30 +321,26 @@ Mutex::Mutex(const char* name, LockLevel level, bool recursive) exclusive_owner_ = 0; } -// Helper to ignore the lock requirement. -static bool IsShuttingDown() NO_THREAD_SAFETY_ANALYSIS { - Runtime* runtime = Runtime::Current(); - return runtime == nullptr || runtime->IsShuttingDownLocked(); +// Helper to allow checking shutdown while locking for thread safety. +static bool IsSafeToCallAbortSafe() { + MutexLock mu(Thread::Current(), *Locks::runtime_shutdown_lock_); + return Locks::IsSafeToCallAbortRacy(); } Mutex::~Mutex() { - bool shutting_down = IsShuttingDown(); + bool safe_to_call_abort = Locks::IsSafeToCallAbortRacy(); #if ART_USE_FUTEXES if (state_.LoadRelaxed() != 0) { - LOG(shutting_down - ? ::android::base::WARNING - : ::android::base::FATAL) << "destroying mutex with owner: " << exclusive_owner_; + LOG(safe_to_call_abort ? FATAL : WARNING) + << "destroying mutex with owner: " << exclusive_owner_; } else { if (exclusive_owner_ != 0) { - LOG(shutting_down - ? ::android::base::WARNING - : ::android::base::FATAL) << "unexpectedly found an owner on unlocked mutex " - << name_; + LOG(safe_to_call_abort ? FATAL : WARNING) + << "unexpectedly found an owner on unlocked mutex " << name_; } if (num_contenders_.LoadSequentiallyConsistent() != 0) { - LOG(shutting_down - ? ::android::base::WARNING - : ::android::base::FATAL) << "unexpectedly found a contender on mutex " << name_; + LOG(safe_to_call_abort ? FATAL : WARNING) + << "unexpectedly found a contender on mutex " << name_; } } #else @@ -352,11 +349,8 @@ Mutex::~Mutex() { int rc = pthread_mutex_destroy(&mutex_); if (rc != 0) { errno = rc; - // TODO: should we just not log at all if shutting down? this could be the logging mutex! - MutexLock mu(Thread::Current(), *Locks::runtime_shutdown_lock_); - PLOG(shutting_down - ? ::android::base::WARNING - : ::android::base::FATAL) << "pthread_mutex_destroy failed for " << name_; + PLOG(safe_to_call_abort ? FATAL : WARNING) + << "pthread_mutex_destroy failed for " << name_; } #endif } @@ -544,11 +538,8 @@ ReaderWriterMutex::~ReaderWriterMutex() { int rc = pthread_rwlock_destroy(&rwlock_); if (rc != 0) { errno = rc; - // TODO: should we just not log at all if shutting down? this could be the logging mutex! - MutexLock mu(Thread::Current(), *Locks::runtime_shutdown_lock_); - Runtime* runtime = Runtime::Current(); - bool shutting_down = runtime == nullptr || runtime->IsShuttingDownLocked(); - PLOG(shutting_down ? WARNING : FATAL) << "pthread_rwlock_destroy failed for " << name_; + bool is_safe_to_call_abort = IsSafeToCallAbortSafe(); + PLOG(is_safe_to_call_abort ? FATAL : WARNING) << "pthread_rwlock_destroy failed for " << name_; } #endif } @@ -772,11 +763,8 @@ ConditionVariable::ConditionVariable(const char* name, Mutex& guard) ConditionVariable::~ConditionVariable() { #if ART_USE_FUTEXES if (num_waiters_!= 0) { - Runtime* runtime = Runtime::Current(); - bool shutting_down = runtime == nullptr || runtime->IsShuttingDown(Thread::Current()); - LOG(shutting_down - ? ::android::base::WARNING - : ::android::base::FATAL) + bool is_safe_to_call_abort = IsSafeToCallAbortSafe(); + LOG(is_safe_to_call_abort ? FATAL : WARNING) << "ConditionVariable::~ConditionVariable for " << name_ << " called with " << num_waiters_ << " waiters."; } @@ -786,12 +774,8 @@ ConditionVariable::~ConditionVariable() { int rc = pthread_cond_destroy(&cond_); if (rc != 0) { errno = rc; - MutexLock mu(Thread::Current(), *Locks::runtime_shutdown_lock_); - Runtime* runtime = Runtime::Current(); - bool shutting_down = (runtime == nullptr) || runtime->IsShuttingDownLocked(); - PLOG(shutting_down - ? ::android::base::WARNING - : ::android::base::FATAL) << "pthread_cond_destroy failed for " << name_; + bool is_safe_to_call_abort = IsSafeToCallAbortSafe(); + PLOG(is_safe_to_call_abort ? FATAL : WARNING) << "pthread_cond_destroy failed for " << name_; } #endif } @@ -1129,4 +1113,14 @@ void Locks::InitConditions() { thread_exit_cond_ = new ConditionVariable("thread exit condition variable", *thread_list_lock_); } +void Locks::SetClientCallback(ClientCallback* safe_to_call_abort_cb) { + safe_to_call_abort_callback.StoreRelease(safe_to_call_abort_cb); +} + +// Helper to allow checking shutdown while ignoring locking requirements. +bool Locks::IsSafeToCallAbortRacy() { + Locks::ClientCallback* safe_to_call_abort_cb = safe_to_call_abort_callback.LoadAcquire(); + return safe_to_call_abort_cb != nullptr && safe_to_call_abort_cb(); +} + } // namespace art diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h index e0cca7b0ce..3f2c5a9e74 100644 --- a/runtime/base/mutex.h +++ b/runtime/base/mutex.h @@ -558,6 +558,18 @@ class Locks { public: static void Init(); static void InitConditions() NO_THREAD_SAFETY_ANALYSIS; // Condition variables. + + // Destroying various lock types can emit errors that vary depending upon + // whether the client (art::Runtime) is currently active. Allow the client + // to set a callback that is used to check when it is acceptable to call + // Abort. The default behavior is that the client *is not* able to call + // Abort if no callback is established. + using ClientCallback = bool(); + static void SetClientCallback(ClientCallback* is_safe_to_call_abort_cb) NO_THREAD_SAFETY_ANALYSIS; + // Checks for whether it is safe to call Abort() without using locks. + static bool IsSafeToCallAbortRacy() NO_THREAD_SAFETY_ANALYSIS; + + // Guards allocation entrypoint instrumenting. static Mutex* instrument_entrypoints_lock_; diff --git a/runtime/runtime.cc b/runtime/runtime.cc index bde41858cf..e8f41d4a55 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -508,12 +508,21 @@ bool Runtime::ParseOptions(const RuntimeOptions& raw_options, return true; } +// Callback to check whether it is safe to call Abort (e.g., to use a call to +// LOG(FATAL)). It is only safe to call Abort if the runtime has been created, +// properly initialized, and has not shut down. +static bool IsSafeToCallAbort() NO_THREAD_SAFETY_ANALYSIS { + Runtime* runtime = Runtime::Current(); + return runtime != nullptr && runtime->IsStarted() && !runtime->IsShuttingDownLocked(); +} + bool Runtime::Create(RuntimeArgumentMap&& runtime_options) { // TODO: acquire a static mutex on Runtime to avoid racing. if (Runtime::instance_ != nullptr) { return false; } instance_ = new Runtime; + Locks::SetClientCallback(IsSafeToCallAbort); if (!instance_->Init(std::move(runtime_options))) { // TODO: Currently deleting the instance will abort the runtime on destruction. Now This will // leak memory, instead. Fix the destructor. b/19100793. diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h index 6c3811b8e5..5fa9353207 100644 --- a/runtime/thread-inl.h +++ b/runtime/thread-inl.h @@ -29,6 +29,7 @@ #include "base/mutex-inl.h" #include "gc/heap.h" #include "jni_env_ext.h" +#include "runtime.h" #include "thread_pool.h" namespace art { |