ART: Fix RequestSynchronousCheckpoint
Fix the known races in the code by requiring more locks on entry.
This helps ensure there is no time in the caller where the thread
can die while requesting the checkpoint.
Fix up GetStackTrace, GetFrameCount and GetFrameLocation.
Bug: 62353392
Test: m test-art-host
Test: art/test/testrunner/testrunner.py -b --host -t 911
Change-Id: Ia0c5e920599b58098dc4b3a3d09c5284045f2947
diff --git a/runtime/openjdkjvmti/ti_stack.cc b/runtime/openjdkjvmti/ti_stack.cc
index 9f4002d..ee89372 100644
--- a/runtime/openjdkjvmti/ti_stack.cc
+++ b/runtime/openjdkjvmti/ti_stack.cc
@@ -205,7 +205,12 @@
size_t index = 0;
};
-static jvmtiError GetThread(JNIEnv* env, jthread java_thread, art::Thread** thread) {
+static jvmtiError GetThread(JNIEnv* env,
+ art::ScopedObjectAccessAlreadyRunnable& soa,
+ jthread java_thread,
+ art::Thread** thread)
+ REQUIRES_SHARED(art::Locks::mutator_lock_) // Needed for FromManagedThread.
+ REQUIRES(art::Locks::thread_list_lock_) { // Needed for FromManagedThread.
if (java_thread == nullptr) {
*thread = art::Thread::Current();
if (*thread == nullptr) {
@@ -220,8 +225,6 @@
}
// TODO: Need non-aborting call here, to return JVMTI_ERROR_INVALID_THREAD.
- art::ScopedObjectAccess soa(art::Thread::Current());
- art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_);
*thread = art::Thread::FromManagedThread(soa, java_thread);
if (*thread == nullptr) {
return ERR(THREAD_NOT_ALIVE);
@@ -236,8 +239,16 @@
jint max_frame_count,
jvmtiFrameInfo* frame_buffer,
jint* count_ptr) {
+ // It is not great that we have to hold these locks for so long, but it is necessary to ensure
+ // that the thread isn't dying on us.
+ art::ScopedObjectAccess soa(art::Thread::Current());
+ art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_);
+
art::Thread* thread;
- jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(), java_thread, &thread);
+ jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(),
+ soa,
+ java_thread,
+ &thread);
if (thread_error != ERR(NONE)) {
return thread_error;
}
@@ -675,8 +686,17 @@
jvmtiError StackUtil::GetFrameCount(jvmtiEnv* env ATTRIBUTE_UNUSED,
jthread java_thread,
jint* count_ptr) {
+ // It is not great that we have to hold these locks for so long, but it is necessary to ensure
+ // that the thread isn't dying on us.
+ art::ScopedObjectAccess soa(art::Thread::Current());
+ art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_);
+
art::Thread* thread;
- jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(), java_thread, &thread);
+ jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(),
+ soa,
+ java_thread,
+ &thread);
+
if (thread_error != ERR(NONE)) {
return thread_error;
}
@@ -745,8 +765,16 @@
jint depth,
jmethodID* method_ptr,
jlocation* location_ptr) {
+ // It is not great that we have to hold these locks for so long, but it is necessary to ensure
+ // that the thread isn't dying on us.
+ art::ScopedObjectAccess soa(art::Thread::Current());
+ art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_);
+
art::Thread* thread;
- jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(), java_thread, &thread);
+ jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(),
+ soa,
+ java_thread,
+ &thread);
if (thread_error != ERR(NONE)) {
return thread_error;
}