Add Jvmti Suspend/ResumeThread functions
Enable the can_suspend jvmti capability and implement all required
functionality associated with it.
Test: ./test.py --host -j40
Bug: 34415266
Bug: 62821960
Bug: 63579748
Change-Id: I83b92de7f81622e1658114b034918e8295805b6e
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index b0394a5..a472b67 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -68,6 +68,7 @@
Mutex* Locks::thread_suspend_count_lock_ = nullptr;
Mutex* Locks::trace_lock_ = nullptr;
Mutex* Locks::unexpected_signal_lock_ = nullptr;
+Mutex* Locks::user_code_suspension_lock_ = nullptr;
Uninterruptible Roles::uninterruptible_;
ReaderWriterMutex* Locks::jni_globals_lock_ = nullptr;
Mutex* Locks::jni_weak_globals_lock_ = nullptr;
@@ -1029,6 +1030,7 @@
DCHECK(thread_suspend_count_lock_ != nullptr);
DCHECK(trace_lock_ != nullptr);
DCHECK(unexpected_signal_lock_ != nullptr);
+ DCHECK(user_code_suspension_lock_ != nullptr);
DCHECK(dex_lock_ != nullptr);
} else {
// Create global locks in level order from highest lock level to lowest.
@@ -1045,6 +1047,10 @@
} \
current_lock_level = new_level;
+ UPDATE_CURRENT_LOCK_LEVEL(kUserCodeSuspensionLock);
+ DCHECK(user_code_suspension_lock_ == nullptr);
+ user_code_suspension_lock_ = new Mutex("user code suspension lock", current_lock_level);
+
UPDATE_CURRENT_LOCK_LEVEL(kMutatorLock);
DCHECK(mutator_lock_ == nullptr);
mutator_lock_ = new MutatorMutex("mutator lock", current_lock_level);
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index e77d8d7..7a472e7 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -116,6 +116,7 @@
kTraceLock,
kHeapBitmapLock,
kMutatorLock,
+ kUserCodeSuspensionLock,
kInstrumentEntrypointsLock,
kZygoteCreationLock,
@@ -578,6 +579,11 @@
// Guards allocation entrypoint instrumenting.
static Mutex* instrument_entrypoints_lock_;
+ // Guards code that deals with user-code suspension. This mutex must be held when suspending or
+ // resuming threads with SuspendReason::kForUserCode. It may be held by a suspended thread, but
+ // only if the suspension is not due to SuspendReason::kForUserCode.
+ static Mutex* user_code_suspension_lock_ ACQUIRED_AFTER(instrument_entrypoints_lock_);
+
// A barrier is used to synchronize the GC/Debugger thread with mutator threads. When GC/Debugger
// thread wants to suspend all mutator threads, it needs to wait for all mutator threads to pass
// a barrier. Threads that are already suspended will get their barrier passed by the GC/Debugger
@@ -613,7 +619,7 @@
// state is changed | .. running ..
// - if the CAS operation fails then goto x | .. running ..
// .. running .. | .. running ..
- static MutatorMutex* mutator_lock_ ACQUIRED_AFTER(instrument_entrypoints_lock_);
+ static MutatorMutex* mutator_lock_ ACQUIRED_AFTER(user_code_suspension_lock_);
// Allow reader-writer mutual exclusion on the mark and live bitmaps of the heap.
static ReaderWriterMutex* heap_bitmap_lock_ ACQUIRED_AFTER(mutator_lock_);
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index b595112..7fcc28c 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -2480,7 +2480,8 @@
needs_resume = thread->GetDebugSuspendCount() > 0;
}
if (needs_resume) {
- Runtime::Current()->GetThreadList()->Resume(thread, SuspendReason::kForDebugger);
+ bool resumed = Runtime::Current()->GetThreadList()->Resume(thread, SuspendReason::kForDebugger);
+ DCHECK(resumed);
}
}
@@ -3721,7 +3722,9 @@
~ScopedDebuggerThreadSuspension() {
if (other_suspend_) {
- Runtime::Current()->GetThreadList()->Resume(thread_, SuspendReason::kForDebugger);
+ bool resumed = Runtime::Current()->GetThreadList()->Resume(thread_,
+ SuspendReason::kForDebugger);
+ DCHECK(resumed);
}
}
@@ -4043,7 +4046,8 @@
thread_list->UndoDebuggerSuspensions();
} else {
VLOG(jdwp) << " Resuming event thread only";
- thread_list->Resume(targetThread, SuspendReason::kForDebugger);
+ bool resumed = thread_list->Resume(targetThread, SuspendReason::kForDebugger);
+ DCHECK(resumed);
}
return JDWP::ERR_NONE;
diff --git a/runtime/monitor.cc b/runtime/monitor.cc
index 3e3eaae..5c63dca 100644
--- a/runtime/monitor.cc
+++ b/runtime/monitor.cc
@@ -910,7 +910,8 @@
// Go ahead and inflate the lock.
Inflate(self, owner, obj.Get(), hash_code);
}
- thread_list->Resume(owner, SuspendReason::kInternal);
+ bool resumed = thread_list->Resume(owner, SuspendReason::kInternal);
+ DCHECK(resumed);
}
self->SetMonitorEnterObject(nullptr);
}
diff --git a/runtime/native/dalvik_system_VMStack.cc b/runtime/native/dalvik_system_VMStack.cc
index 7d2d0e5..2aeef60 100644
--- a/runtime/native/dalvik_system_VMStack.cc
+++ b/runtime/native/dalvik_system_VMStack.cc
@@ -62,7 +62,8 @@
trace = thread->CreateInternalStackTrace<false>(soa);
}
// Restart suspended thread.
- thread_list->Resume(thread, SuspendReason::kInternal);
+ bool resumed = thread_list->Resume(thread, SuspendReason::kInternal);
+ DCHECK(resumed);
} else if (timed_out) {
LOG(ERROR) << "Trying to get thread's stack failed as the thread failed to suspend within a "
"generous timeout.";
diff --git a/runtime/native/java_lang_Thread.cc b/runtime/native/java_lang_Thread.cc
index 8b76327..4ce72ed 100644
--- a/runtime/native/java_lang_Thread.cc
+++ b/runtime/native/java_lang_Thread.cc
@@ -155,7 +155,8 @@
ScopedObjectAccess soa(env);
thread->SetThreadName(name.c_str());
}
- thread_list->Resume(thread, SuspendReason::kInternal);
+ bool resumed = thread_list->Resume(thread, SuspendReason::kInternal);
+ DCHECK(resumed);
} else if (timed_out) {
LOG(ERROR) << "Trying to set thread name to '" << name.c_str() << "' failed as the thread "
"failed to suspend within a generous timeout.";
diff --git a/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc b/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc
index c516b66..125d737 100644
--- a/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc
+++ b/runtime/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc
@@ -76,7 +76,8 @@
trace = Thread::InternalStackTraceToStackTraceElementArray(soa, internal_trace);
}
// Restart suspended thread.
- thread_list->Resume(thread, SuspendReason::kInternal);
+ bool resumed = thread_list->Resume(thread, SuspendReason::kInternal);
+ DCHECK(resumed);
} else {
if (timed_out) {
LOG(ERROR) << "Trying to get thread's stack by id failed as the thread failed to suspend "
diff --git a/runtime/openjdkjvm/OpenjdkJvm.cc b/runtime/openjdkjvm/OpenjdkJvm.cc
index 4560dda..6a8f2ce 100644
--- a/runtime/openjdkjvm/OpenjdkJvm.cc
+++ b/runtime/openjdkjvm/OpenjdkJvm.cc
@@ -432,7 +432,8 @@
art::ScopedObjectAccess soa(env);
thread->SetThreadName(name.c_str());
}
- thread_list->Resume(thread, art::SuspendReason::kInternal);
+ bool resumed = thread_list->Resume(thread, art::SuspendReason::kInternal);
+ DCHECK(resumed);
} else if (timed_out) {
LOG(ERROR) << "Trying to set thread name to '" << name.c_str() << "' failed as the thread "
"failed to suspend within a generous timeout.";
diff --git a/runtime/openjdkjvmti/OpenjdkJvmTi.cc b/runtime/openjdkjvmti/OpenjdkJvmTi.cc
index 505e844..4dff660 100644
--- a/runtime/openjdkjvmti/OpenjdkJvmTi.cc
+++ b/runtime/openjdkjvmti/OpenjdkJvmTi.cc
@@ -133,34 +133,34 @@
return ThreadUtil::GetAllThreads(env, threads_count_ptr, threads_ptr);
}
- static jvmtiError SuspendThread(jvmtiEnv* env, jthread thread ATTRIBUTE_UNUSED) {
+ static jvmtiError SuspendThread(jvmtiEnv* env, jthread thread) {
ENSURE_VALID_ENV(env);
ENSURE_HAS_CAP(env, can_suspend);
- return ERR(NOT_IMPLEMENTED);
+ return ThreadUtil::SuspendThread(env, thread);
}
static jvmtiError SuspendThreadList(jvmtiEnv* env,
- jint request_count ATTRIBUTE_UNUSED,
- const jthread* request_list ATTRIBUTE_UNUSED,
- jvmtiError* results ATTRIBUTE_UNUSED) {
+ jint request_count,
+ const jthread* request_list,
+ jvmtiError* results) {
ENSURE_VALID_ENV(env);
ENSURE_HAS_CAP(env, can_suspend);
- return ERR(NOT_IMPLEMENTED);
+ return ThreadUtil::SuspendThreadList(env, request_count, request_list, results);
}
- static jvmtiError ResumeThread(jvmtiEnv* env, jthread thread ATTRIBUTE_UNUSED) {
+ static jvmtiError ResumeThread(jvmtiEnv* env, jthread thread) {
ENSURE_VALID_ENV(env);
ENSURE_HAS_CAP(env, can_suspend);
- return ERR(NOT_IMPLEMENTED);
+ return ThreadUtil::ResumeThread(env, thread);
}
static jvmtiError ResumeThreadList(jvmtiEnv* env,
- jint request_count ATTRIBUTE_UNUSED,
- const jthread* request_list ATTRIBUTE_UNUSED,
- jvmtiError* results ATTRIBUTE_UNUSED) {
+ jint request_count,
+ const jthread* request_list,
+ jvmtiError* results) {
ENSURE_VALID_ENV(env);
ENSURE_HAS_CAP(env, can_suspend);
- return ERR(NOT_IMPLEMENTED);
+ return ThreadUtil::ResumeThreadList(env, request_count, request_list, results);
}
static jvmtiError StopThread(jvmtiEnv* env,
diff --git a/runtime/openjdkjvmti/art_jvmti.h b/runtime/openjdkjvmti/art_jvmti.h
index c63e502..4d5bb95 100644
--- a/runtime/openjdkjvmti/art_jvmti.h
+++ b/runtime/openjdkjvmti/art_jvmti.h
@@ -233,7 +233,7 @@
.can_generate_exception_events = 0,
.can_generate_frame_pop_events = 0,
.can_generate_breakpoint_events = 1,
- .can_suspend = 0,
+ .can_suspend = 1,
.can_redefine_any_class = 0,
.can_get_current_thread_cpu_time = 0,
.can_get_thread_cpu_time = 0,
diff --git a/runtime/openjdkjvmti/ti_thread.cc b/runtime/openjdkjvmti/ti_thread.cc
index 2cc2a26..3d447dc 100644
--- a/runtime/openjdkjvmti/ti_thread.cc
+++ b/runtime/openjdkjvmti/ti_thread.cc
@@ -433,6 +433,9 @@
if (native_thread->IsInterrupted()) {
jvmti_state |= JVMTI_THREAD_STATE_INTERRUPTED;
}
+ if (native_thread->IsSuspended()) {
+ jvmti_state |= JVMTI_THREAD_STATE_SUSPENDED;
+ }
// Java state is derived from nativeGetState.
// Note: Our implementation assigns "runnable" to suspended. As such, we will have slightly
@@ -605,4 +608,200 @@
return ERR(NONE);
}
+// Suspends the current thread if it has any suspend requests on it.
+static void SuspendCheck(art::Thread* self)
+ REQUIRES(!art::Locks::mutator_lock_, !art::Locks::user_code_suspension_lock_) {
+ art::ScopedObjectAccess soa(self);
+ // Really this is only needed if we are in FastJNI and actually have the mutator_lock_ already.
+ self->FullSuspendCheck();
+}
+
+jvmtiError ThreadUtil::SuspendOther(art::Thread* self,
+ jthread target_jthread,
+ art::Thread* target) {
+ // Loop since we need to bail out and try again if we would end up getting suspended while holding
+ // the user_code_suspension_lock_ due to a SuspendReason::kForUserCode. In this situation we
+ // release the lock, wait to get resumed and try again.
+ do {
+ // Suspend ourself if we have any outstanding suspends. This is so we won't suspend due to
+ // another SuspendThread in the middle of suspending something else potentially causing a
+ // deadlock. We need to do this in the loop because if we ended up back here then we had
+ // outstanding SuspendReason::kForUserCode suspensions and we should wait for them to be cleared
+ // before continuing.
+ SuspendCheck(self);
+ art::MutexLock mu(self, *art::Locks::user_code_suspension_lock_);
+ {
+ art::MutexLock thread_list_mu(self, *art::Locks::thread_suspend_count_lock_);
+ // Make sure we won't be suspended in the middle of holding the thread_suspend_count_lock_ by
+ // a user-code suspension. We retry and do another SuspendCheck to clear this.
+ if (self->GetUserCodeSuspendCount() != 0) {
+ continue;
+ } else if (target->GetUserCodeSuspendCount() != 0) {
+ return ERR(THREAD_SUSPENDED);
+ }
+ }
+ bool timeout = true;
+ while (timeout) {
+ art::ThreadState state = target->GetState();
+ if (state == art::ThreadState::kTerminated || state == art::ThreadState::kStarting) {
+ return ERR(THREAD_NOT_ALIVE);
+ }
+ target = art::Runtime::Current()->GetThreadList()->SuspendThreadByPeer(
+ target_jthread,
+ /* request_suspension */ true,
+ art::SuspendReason::kForUserCode,
+ &timeout);
+ if (target == nullptr && !timeout) {
+ // TODO It would be good to get more information about why exactly the thread failed to
+ // suspend.
+ return ERR(INTERNAL);
+ }
+ }
+ return OK;
+ } while (true);
+ UNREACHABLE();
+}
+
+jvmtiError ThreadUtil::SuspendSelf(art::Thread* self) {
+ CHECK(self == art::Thread::Current());
+ {
+ art::MutexLock mu(self, *art::Locks::user_code_suspension_lock_);
+ art::MutexLock thread_list_mu(self, *art::Locks::thread_suspend_count_lock_);
+ if (self->GetUserCodeSuspendCount() != 0) {
+ // This can only happen if we race with another thread to suspend 'self' and we lose.
+ return ERR(THREAD_SUSPENDED);
+ }
+ // We shouldn't be able to fail this.
+ if (!self->ModifySuspendCount(self, +1, nullptr, art::SuspendReason::kForUserCode)) {
+ // TODO More specific error would be nice.
+ return ERR(INTERNAL);
+ }
+ }
+ // Once we have requested the suspend we actually go to sleep. We need to do this after releasing
+ // the suspend_lock to make sure we can be woken up. This call gains the mutator lock causing us
+ // to go to sleep until we are resumed.
+ SuspendCheck(self);
+ return OK;
+}
+
+jvmtiError ThreadUtil::SuspendThread(jvmtiEnv* env ATTRIBUTE_UNUSED, jthread thread) {
+ art::Thread* self = art::Thread::Current();
+ art::Thread* target;
+ {
+ art::ScopedObjectAccess soa(self);
+ target = GetNativeThread(thread, soa);
+ }
+ if (target == nullptr) {
+ return ERR(INVALID_THREAD);
+ }
+ if (target == self) {
+ return SuspendSelf(self);
+ } else {
+ return SuspendOther(self, thread, target);
+ }
+}
+
+jvmtiError ThreadUtil::ResumeThread(jvmtiEnv* env ATTRIBUTE_UNUSED,
+ jthread thread) {
+ if (thread == nullptr) {
+ return ERR(NULL_POINTER);
+ }
+ art::Thread* self = art::Thread::Current();
+ art::Thread* target;
+ {
+ // NB This does a SuspendCheck (during thread state change) so we need to make sure we don't
+ // have the 'suspend_lock' locked here.
+ art::ScopedObjectAccess soa(self);
+ target = GetNativeThread(thread, soa);
+ }
+ if (target == nullptr) {
+ return ERR(INVALID_THREAD);
+ } else if (target == self) {
+ // We would have paused until we aren't suspended anymore due to the ScopedObjectAccess so we
+ // can just return THREAD_NOT_SUSPENDED. Unfortunately we cannot do any real DCHECKs about
+ // current state since it's all concurrent.
+ return ERR(THREAD_NOT_SUSPENDED);
+ }
+ // Now that we know we aren't getting suspended ourself (since we have a mutator lock) we lock the
+ // suspend_lock to start suspending.
+ art::MutexLock mu(self, *art::Locks::user_code_suspension_lock_);
+ {
+ // The JVMTI spec requires us to return THREAD_NOT_SUSPENDED if it is alive but we really cannot
+ // tell why resume failed.
+ art::MutexLock thread_list_mu(self, *art::Locks::thread_suspend_count_lock_);
+ if (target->GetUserCodeSuspendCount() == 0) {
+ return ERR(THREAD_NOT_SUSPENDED);
+ }
+ }
+ if (target->GetState() == art::ThreadState::kTerminated) {
+ return ERR(THREAD_NOT_ALIVE);
+ }
+ DCHECK(target != self);
+ if (!art::Runtime::Current()->GetThreadList()->Resume(target, art::SuspendReason::kForUserCode)) {
+ // TODO Give a better error.
+ // This is most likely THREAD_NOT_SUSPENDED but we cannot really be sure.
+ return ERR(INTERNAL);
+ }
+ return OK;
+}
+
+// Suspends all the threads in the list at the same time. Getting this behavior is a little tricky
+// since we can have threads in the list multiple times. This generally doesn't matter unless the
+// current thread is present multiple times. In that case we need to suspend only once and either
+// return the same error code in all the other slots if it failed or return ERR(THREAD_SUSPENDED) if
+// it didn't. We also want to handle the current thread last to make the behavior of the code
+// simpler to understand.
+jvmtiError ThreadUtil::SuspendThreadList(jvmtiEnv* env,
+ jint request_count,
+ const jthread* threads,
+ jvmtiError* results) {
+ if (request_count == 0) {
+ return ERR(ILLEGAL_ARGUMENT);
+ } else if (results == nullptr || threads == nullptr) {
+ return ERR(NULL_POINTER);
+ }
+ // This is the list of the indexes in 'threads' and 'results' that correspond to the currently
+ // running thread. These indexes we need to handle specially since we need to only actually
+ // suspend a single time.
+ std::vector<jint> current_thread_indexes;
+ art::Thread* self = art::Thread::Current();
+ for (jint i = 0; i < request_count; i++) {
+ {
+ art::ScopedObjectAccess soa(self);
+ if (threads[i] == nullptr || GetNativeThread(threads[i], soa) == self) {
+ current_thread_indexes.push_back(i);
+ continue;
+ }
+ }
+ results[i] = env->SuspendThread(threads[i]);
+ }
+ if (!current_thread_indexes.empty()) {
+ jint first_current_thread_index = current_thread_indexes[0];
+ // Suspend self.
+ jvmtiError res = env->SuspendThread(threads[first_current_thread_index]);
+ results[first_current_thread_index] = res;
+ // Fill in the rest of the error values as appropriate.
+ jvmtiError other_results = (res != OK) ? res : ERR(THREAD_SUSPENDED);
+ for (auto it = ++current_thread_indexes.begin(); it != current_thread_indexes.end(); ++it) {
+ results[*it] = other_results;
+ }
+ }
+ return OK;
+}
+
+jvmtiError ThreadUtil::ResumeThreadList(jvmtiEnv* env,
+ jint request_count,
+ const jthread* threads,
+ jvmtiError* results) {
+ if (request_count == 0) {
+ return ERR(ILLEGAL_ARGUMENT);
+ } else if (results == nullptr || threads == nullptr) {
+ return ERR(NULL_POINTER);
+ }
+ for (jint i = 0; i < request_count; i++) {
+ results[i] = env->ResumeThread(threads[i]);
+ }
+ return OK;
+}
+
} // namespace openjdkjvmti
diff --git a/runtime/openjdkjvmti/ti_thread.h b/runtime/openjdkjvmti/ti_thread.h
index 939aea7..57967eb 100644
--- a/runtime/openjdkjvmti/ti_thread.h
+++ b/runtime/openjdkjvmti/ti_thread.h
@@ -35,8 +35,11 @@
#include "jni.h"
#include "jvmti.h"
+#include "base/mutex.h"
+
namespace art {
class ArtField;
+class Thread;
} // namespace art
namespace openjdkjvmti {
@@ -68,7 +71,33 @@
const void* arg,
jint priority);
+ static jvmtiError SuspendThread(jvmtiEnv* env, jthread thread);
+ static jvmtiError ResumeThread(jvmtiEnv* env, jthread thread);
+
+ static jvmtiError SuspendThreadList(jvmtiEnv* env,
+ jint request_count,
+ const jthread* threads,
+ jvmtiError* results);
+ static jvmtiError ResumeThreadList(jvmtiEnv* env,
+ jint request_count,
+ const jthread* threads,
+ jvmtiError* results);
+
private:
+ // We need to make sure only one thread tries to suspend threads at a time so we can get the
+ // 'suspend-only-once' behavior the spec requires. Internally, ART considers suspension to be a
+ // counted state, allowing a single thread to be suspended multiple times by different users. This
+ // makes mapping into the JVMTI idea of thread suspension difficult. We have decided to split the
+ // difference and ensure that JVMTI tries to treat suspension as the boolean flag as much as
+ // possible with the suspend/resume methods but only do best effort. On the other hand
+ // GetThreadState will be totally accurate as much as possible. This means that calling
+ // ResumeThread on a thread that has state JVMTI_THREAD_STATE_SUSPENDED will not necessarily
+ // cause the thread to wake up if the thread is suspended for the debugger or gc or something.
+ static jvmtiError SuspendSelf(art::Thread* self)
+ REQUIRES(!art::Locks::mutator_lock_, !art::Locks::user_code_suspension_lock_);
+ static jvmtiError SuspendOther(art::Thread* self, jthread target_jthread, art::Thread* target)
+ REQUIRES(!art::Locks::mutator_lock_, !art::Locks::user_code_suspension_lock_);
+
static art::ArtField* context_class_loader_;
};
diff --git a/runtime/suspend_reason.h b/runtime/suspend_reason.h
index 27c4d32..289a1a4 100644
--- a/runtime/suspend_reason.h
+++ b/runtime/suspend_reason.h
@@ -28,6 +28,8 @@
kInternal,
// Suspending for debugger (code in Dbg::*, runtime/jdwp/, etc.).
kForDebugger,
+ // Suspending due to non-runtime, user controlled, code. (For example Thread#Suspend()).
+ kForUserCode,
};
std::ostream& operator<<(std::ostream& os, const SuspendReason& thread);
diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h
index 95608b5..b5a9626 100644
--- a/runtime/thread-inl.h
+++ b/runtime/thread-inl.h
@@ -121,10 +121,20 @@
return false;
}
for (int i = kLockLevelCount - 1; i >= 0; --i) {
- if (i != kMutatorLock && GetHeldMutex(static_cast<LockLevel>(i)) != nullptr) {
+ if (i != kMutatorLock &&
+ i != kUserCodeSuspensionLock &&
+ GetHeldMutex(static_cast<LockLevel>(i)) != nullptr) {
return false;
}
}
+ // Thread autoanalysis isn't able to understand that the GetHeldMutex(...) or AssertHeld means we
+ // have the mutex meaning we need to do this hack.
+ auto is_suspending_for_user_code = [this]() NO_THREAD_SAFETY_ANALYSIS {
+ return tls32_.user_code_suspend_count != 0;
+ };
+ if (GetHeldMutex(kUserCodeSuspensionLock) != nullptr && is_suspending_for_user_code()) {
+ return false;
+ }
return true;
}
@@ -136,8 +146,9 @@
if (check_locks) {
bool bad_mutexes_held = false;
for (int i = kLockLevelCount - 1; i >= 0; --i) {
- // We expect no locks except the mutator_lock_ or thread list suspend thread lock.
- if (i != kMutatorLock) {
+ // We expect no locks except the mutator_lock_. User code suspension lock is OK as long as
+ // we aren't going to be held suspended due to SuspendReason::kForUserCode.
+ if (i != kMutatorLock && i != kUserCodeSuspensionLock) {
BaseMutex* held_mutex = GetHeldMutex(static_cast<LockLevel>(i));
if (held_mutex != nullptr) {
LOG(ERROR) << "holding \"" << held_mutex->GetName()
@@ -146,6 +157,19 @@
}
}
}
+ // Make sure that if we hold the user_code_suspension_lock_ we aren't suspending due to
+ // user_code_suspend_count which would prevent the thread from ever waking up. Thread
+ // autoanalysis isn't able to understand that the GetHeldMutex(...) or AssertHeld means we
+ // have the mutex meaning we need to do this hack.
+ auto is_suspending_for_user_code = [this]() NO_THREAD_SAFETY_ANALYSIS {
+ return tls32_.user_code_suspend_count != 0;
+ };
+ if (GetHeldMutex(kUserCodeSuspensionLock) != nullptr && is_suspending_for_user_code()) {
+ LOG(ERROR) << "suspending due to user-code while holding \""
+ << Locks::user_code_suspension_lock_->GetName() << "\"! Thread would never "
+ << "wake up.";
+ bad_mutexes_held = true;
+ }
if (gAborting == 0) {
CHECK(!bad_mutexes_held);
}
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 36ecd33..6a3f9e7 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1208,6 +1208,15 @@
Locks::thread_list_lock_->AssertHeld(self);
}
}
+ // User code suspensions need to be checked more closely since they originate from code outside of
+ // the runtime's control.
+ if (UNLIKELY(reason == SuspendReason::kForUserCode)) {
+ Locks::user_code_suspension_lock_->AssertHeld(self);
+ if (UNLIKELY(delta + tls32_.user_code_suspend_count < 0)) {
+ LOG(ERROR) << "attempting to modify suspend count in an illegal way.";
+ return false;
+ }
+ }
if (UNLIKELY(delta < 0 && tls32_.suspend_count <= 0)) {
UnsafeLogFatalForSuspendCount(self, this);
return false;
@@ -1241,6 +1250,9 @@
case SuspendReason::kForDebugger:
tls32_.debug_suspend_count += delta;
break;
+ case SuspendReason::kForUserCode:
+ tls32_.user_code_suspend_count += delta;
+ break;
case SuspendReason::kInternal:
break;
}
diff --git a/runtime/thread.h b/runtime/thread.h
index e785ddc..0128c96 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -228,6 +228,11 @@
return tls32_.suspend_count;
}
+ int GetUserCodeSuspendCount() const REQUIRES(Locks::thread_suspend_count_lock_,
+ Locks::user_code_suspension_lock_) {
+ return tls32_.user_code_suspend_count;
+ }
+
int GetDebugSuspendCount() const REQUIRES(Locks::thread_suspend_count_lock_) {
return tls32_.debug_suspend_count;
}
@@ -1381,7 +1386,7 @@
thread_exit_check_count(0), handling_signal_(false),
is_transitioning_to_runnable(false), ready_for_debug_invoke(false),
debug_method_entry_(false), is_gc_marking(false), weak_ref_access_enabled(true),
- disable_thread_flip_count(0) {
+ disable_thread_flip_count(0), user_code_suspend_count(0) {
}
union StateAndFlags state_and_flags;
@@ -1456,6 +1461,12 @@
// levels of (nested) JNI critical sections the thread is in and is used to detect a nested JNI
// critical section enter.
uint32_t disable_thread_flip_count;
+
+ // How much of 'suspend_count_' is by request of user code, used to distinguish threads
+ // suspended by the runtime from those suspended by user code.
+ // This should have GUARDED_BY(Locks::user_code_suspension_lock_) but auto analysis cannot be
+ // told that AssertHeld should be good enough.
+ int user_code_suspend_count GUARDED_BY(Locks::thread_suspend_count_lock_);
} tls32_;
struct PACKED(8) tls_64bit_sized_values {
diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index fc767ed..9c938ff 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -828,7 +828,7 @@
}
}
-void ThreadList::Resume(Thread* thread, SuspendReason reason) {
+bool ThreadList::Resume(Thread* thread, SuspendReason reason) {
// This assumes there was an ATRACE_BEGIN when we suspended the thread.
ATRACE_END();
@@ -841,16 +841,23 @@
MutexLock mu(self, *Locks::thread_list_lock_);
// To check IsSuspended.
MutexLock mu2(self, *Locks::thread_suspend_count_lock_);
- DCHECK(thread->IsSuspended());
+ if (UNLIKELY(!thread->IsSuspended())) {
+ LOG(ERROR) << "Resume(" << reinterpret_cast<void*>(thread)
+ << ") thread not suspended";
+ return false;
+ }
if (!Contains(thread)) {
// We only expect threads within the thread-list to have been suspended otherwise we can't
// stop such threads from delete-ing themselves.
LOG(ERROR) << "Resume(" << reinterpret_cast<void*>(thread)
<< ") thread not within thread list";
- return;
+ return false;
}
- bool updated = thread->ModifySuspendCount(self, -1, nullptr, reason);
- DCHECK(updated);
+ if (UNLIKELY(!thread->ModifySuspendCount(self, -1, nullptr, reason))) {
+ LOG(ERROR) << "Resume(" << reinterpret_cast<void*>(thread)
+ << ") could not modify suspend count.";
+ return false;
+ }
}
{
@@ -860,6 +867,7 @@
}
VLOG(threads) << "Resume(" << reinterpret_cast<void*>(thread) << ") complete";
+ return true;
}
static void ThreadSuspendByPeerWarning(Thread* self,
diff --git a/runtime/thread_list.h b/runtime/thread_list.h
index 41c5e32..11f272c 100644
--- a/runtime/thread_list.h
+++ b/runtime/thread_list.h
@@ -65,8 +65,8 @@
void ResumeAll()
REQUIRES(!Locks::thread_list_lock_, !Locks::thread_suspend_count_lock_)
UNLOCK_FUNCTION(Locks::mutator_lock_);
- void Resume(Thread* thread, SuspendReason reason = SuspendReason::kInternal)
- REQUIRES(!Locks::thread_suspend_count_lock_);
+ bool Resume(Thread* thread, SuspendReason reason = SuspendReason::kInternal)
+ REQUIRES(!Locks::thread_suspend_count_lock_) WARN_UNUSED;
// Suspends all threads and gets exclusive access to the mutator_lock_.
// If long_suspend is true, then other threads who try to suspend will never timeout.