diff options
| -rw-r--r-- | openjdkjvmti/OpenjdkJvmTi.cc | 11 | ||||
| -rw-r--r-- | openjdkjvmti/ti_method.cc | 30 | ||||
| -rw-r--r-- | openjdkjvmti/ti_monitor.cc | 14 | ||||
| -rw-r--r-- | openjdkjvmti/ti_stack.cc | 87 | ||||
| -rw-r--r-- | openjdkjvmti/ti_thread.cc | 134 | ||||
| -rw-r--r-- | openjdkjvmti/ti_thread.h | 20 |
6 files changed, 153 insertions, 143 deletions
diff --git a/openjdkjvmti/OpenjdkJvmTi.cc b/openjdkjvmti/OpenjdkJvmTi.cc index 4339b2bdef..bac57f9bc6 100644 --- a/openjdkjvmti/OpenjdkJvmTi.cc +++ b/openjdkjvmti/OpenjdkJvmTi.cc @@ -1034,14 +1034,13 @@ class JvmtiFunctions { ENSURE_VALID_ENV(env); art::Thread* art_thread = nullptr; if (event_thread != nullptr) { - // TODO: Need non-aborting call here, to return JVMTI_ERROR_INVALID_THREAD. + // TODO The locking around this call is less then what we really want. art::ScopedObjectAccess soa(art::Thread::Current()); art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_); - art_thread = art::Thread::FromManagedThread(soa, event_thread); - - if (art_thread == nullptr || // The thread hasn't been started or is already dead. - art_thread->IsStillStarting()) { - // TODO: We may want to let the EventHandler know, so it could clean up masks, potentially. + jvmtiError err = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(event_thread, soa, &art_thread, &err)) { + return err; + } else if (art_thread->IsStillStarting()) { return ERR(THREAD_NOT_ALIVE); } } diff --git a/openjdkjvmti/ti_method.cc b/openjdkjvmti/ti_method.cc index 62603aa187..625fb26539 100644 --- a/openjdkjvmti/ti_method.cc +++ b/openjdkjvmti/ti_method.cc @@ -761,12 +761,10 @@ jvmtiError MethodUtil::GetLocalVariableGeneric(jvmtiEnv* env ATTRIBUTE_UNUSED, art::jit::ScopedJitSuspend suspend_jit; art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = ThreadUtil::GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); - } - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } GetLocalVariableClosure c(self, depth, slot, type, val); if (!target->RequestSynchronousCheckpoint(&c)) { @@ -890,12 +888,10 @@ jvmtiError MethodUtil::SetLocalVariableGeneric(jvmtiEnv* env ATTRIBUTE_UNUSED, art::jit::ScopedJitSuspend suspend_jit; art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = ThreadUtil::GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); - } - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } SetLocalVariableClosure c(self, depth, slot, type, val); if (!target->RequestSynchronousCheckpoint(&c)) { @@ -955,12 +951,10 @@ jvmtiError MethodUtil::GetLocalInstance(jvmtiEnv* env ATTRIBUTE_UNUSED, art::Thread* self = art::Thread::Current(); art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = ThreadUtil::GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); - } - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } GetLocalInstanceClosure c(self, depth, data); if (!target->RequestSynchronousCheckpoint(&c)) { diff --git a/openjdkjvmti/ti_monitor.cc b/openjdkjvmti/ti_monitor.cc index f92d81ef17..5a38f46901 100644 --- a/openjdkjvmti/ti_monitor.cc +++ b/openjdkjvmti/ti_monitor.cc @@ -335,12 +335,10 @@ jvmtiError MonitorUtil::GetCurrentContendedMonitor(jvmtiEnv* env ATTRIBUTE_UNUSE art::Thread* self = art::Thread::Current(); art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = ThreadUtil::GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); - } - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } struct GetContendedMonitorClosure : public art::Closure { public: @@ -395,7 +393,9 @@ jvmtiError MonitorUtil::GetCurrentContendedMonitor(jvmtiEnv* env ATTRIBUTE_UNUSE jobject* out_; }; GetContendedMonitorClosure closure(self, monitor); - target->RequestSynchronousCheckpoint(&closure); + if (!target->RequestSynchronousCheckpoint(&closure)) { + return ERR(THREAD_NOT_ALIVE); + } return OK; } diff --git a/openjdkjvmti/ti_stack.cc b/openjdkjvmti/ti_stack.cc index 699f6952c4..d4cc42ae70 100644 --- a/openjdkjvmti/ti_stack.cc +++ b/openjdkjvmti/ti_stack.cc @@ -211,34 +211,6 @@ struct GetStackTraceDirectClosure : public art::Closure { size_t index = 0; }; -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) { - // GetStackTrace can only be run during the live phase, so the current thread should be - // attached and thus available. Getting a null for current means we're starting up or - // dying. - return ERR(WRONG_PHASE); - } - } else { - if (!env->IsInstanceOf(java_thread, art::WellKnownClasses::java_lang_Thread)) { - return ERR(INVALID_THREAD); - } - - // TODO: Need non-aborting call here, to return JVMTI_ERROR_INVALID_THREAD. - *thread = art::Thread::FromManagedThread(soa, java_thread); - if (*thread == nullptr) { - return ERR(THREAD_NOT_ALIVE); - } - } - return ERR(NONE); -} - jvmtiError StackUtil::GetStackTrace(jvmtiEnv* jvmti_env ATTRIBUTE_UNUSED, jthread java_thread, jint start_depth, @@ -251,19 +223,14 @@ jvmtiError StackUtil::GetStackTrace(jvmtiEnv* jvmti_env ATTRIBUTE_UNUSED, art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_); art::Thread* thread; - jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(), - soa, - java_thread, - &thread); - if (thread_error != ERR(NONE)) { + jvmtiError thread_error = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(java_thread, soa, &thread, &thread_error)) { return thread_error; } DCHECK(thread != nullptr); art::ThreadState state = thread->GetState(); - if (state == art::ThreadState::kStarting || - state == art::ThreadState::kTerminated || - thread->IsStillStarting()) { + if (state == art::ThreadState::kStarting || thread->IsStillStarting()) { return ERR(THREAD_NOT_ALIVE); } @@ -714,22 +681,25 @@ jvmtiError StackUtil::GetFrameCount(jvmtiEnv* env ATTRIBUTE_UNUSED, art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_); art::Thread* thread; - jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(), - soa, - java_thread, - &thread); - - if (thread_error != ERR(NONE)) { + jvmtiError thread_error = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(java_thread, soa, &thread, &thread_error)) { return thread_error; } + DCHECK(thread != nullptr); + art::ThreadState state = thread->GetState(); + if (state == art::ThreadState::kStarting || thread->IsStillStarting()) { + return ERR(THREAD_NOT_ALIVE); + } if (count_ptr == nullptr) { return ERR(NULL_POINTER); } GetFrameCountClosure closure; - thread->RequestSynchronousCheckpoint(&closure); + if (!thread->RequestSynchronousCheckpoint(&closure)) { + return ERR(THREAD_NOT_ALIVE); + } *count_ptr = closure.count; return ERR(NONE); @@ -793,15 +763,17 @@ jvmtiError StackUtil::GetFrameLocation(jvmtiEnv* env ATTRIBUTE_UNUSED, art::MutexLock mu(soa.Self(), *art::Locks::thread_list_lock_); art::Thread* thread; - jvmtiError thread_error = GetThread(art::Thread::Current()->GetJniEnv(), - soa, - java_thread, - &thread); - if (thread_error != ERR(NONE)) { + jvmtiError thread_error = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(java_thread, soa, &thread, &thread_error)) { return thread_error; } DCHECK(thread != nullptr); + art::ThreadState state = thread->GetState(); + if (state == art::ThreadState::kStarting || thread->IsStillStarting()) { + return ERR(THREAD_NOT_ALIVE); + } + if (depth < 0) { return ERR(ILLEGAL_ARGUMENT); } @@ -920,12 +892,10 @@ static jvmtiError GetOwnedMonitorInfoCommon(jthread thread, Fn handle_results) { bool called_method = false; { art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = ThreadUtil::GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); - } - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } if (target != self) { called_method = true; @@ -1014,10 +984,11 @@ jvmtiError StackUtil::NotifyFramePop(jvmtiEnv* env, jthread thread, jint depth) // have the 'suspend_lock' locked here. art::ScopedObjectAccess soa(self); art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_); - target = ThreadUtil::GetNativeThread(thread, soa); - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); - } else if (target != self) { + jvmtiError err = ERR(INTERNAL); + if (!ThreadUtil::GetAliveNativeThread(thread, soa, &target, &err)) { + return err; + } + if (target != self) { // TODO This is part of the spec but we could easily avoid needing to do it. We would just put // all the logic into a sync-checkpoint. art::MutexLock tscl_mu(self, *art::Locks::thread_suspend_count_lock_); diff --git a/openjdkjvmti/ti_thread.cc b/openjdkjvmti/ti_thread.cc index da1c1bccc7..907b5158c6 100644 --- a/openjdkjvmti/ti_thread.cc +++ b/openjdkjvmti/ti_thread.cc @@ -161,13 +161,34 @@ jvmtiError ThreadUtil::GetCurrentThread(jvmtiEnv* env ATTRIBUTE_UNUSED, jthread* } // Get the native thread. The spec says a null object denotes the current thread. -art::Thread* ThreadUtil::GetNativeThread(jthread thread, - const art::ScopedObjectAccessAlreadyRunnable& soa) { +bool ThreadUtil::GetNativeThread(jthread thread, + const art::ScopedObjectAccessAlreadyRunnable& soa, + /*out*/ art::Thread** thr, + /*out*/ jvmtiError* err) { if (thread == nullptr) { - return art::Thread::Current(); + *thr = art::Thread::Current(); + return true; + } else if (!soa.Env()->IsInstanceOf(thread, art::WellKnownClasses::java_lang_Thread)) { + *err = ERR(INVALID_THREAD); + return false; + } else { + *thr = art::Thread::FromManagedThread(soa, thread); + return true; } +} - return art::Thread::FromManagedThread(soa, thread); +bool ThreadUtil::GetAliveNativeThread(jthread thread, + const art::ScopedObjectAccessAlreadyRunnable& soa, + /*out*/ art::Thread** thr, + /*out*/ jvmtiError* err) { + if (!GetNativeThread(thread, soa, thr, err)) { + return false; + } else if (*thr == nullptr || (*thr)->GetState() == art::ThreadState::kTerminated) { + *err = ERR(THREAD_NOT_ALIVE); + return false; + } else { + return true; + } } jvmtiError ThreadUtil::GetThreadInfo(jvmtiEnv* env, jthread thread, jvmtiThreadInfo* info_ptr) { @@ -182,9 +203,10 @@ jvmtiError ThreadUtil::GetThreadInfo(jvmtiEnv* env, jthread thread, jvmtiThreadI art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); + art::Thread* target; + jvmtiError err = ERR(INTERNAL); + if (!GetNativeThread(thread, soa, &target, &err)) { + return err; } JvmtiUniquePtr<char[]> name_uptr; @@ -297,25 +319,18 @@ struct InternalThreadState { }; // Return the thread's (or current thread, if null) thread state. -static InternalThreadState GetNativeThreadState(jthread thread, - const art::ScopedObjectAccessAlreadyRunnable& soa) +static InternalThreadState GetNativeThreadState(art::Thread* target) REQUIRES_SHARED(art::Locks::mutator_lock_) REQUIRES(art::Locks::thread_list_lock_, art::Locks::user_code_suspension_lock_) { - art::Thread* self = nullptr; - if (thread == nullptr) { - self = art::Thread::Current(); - } else { - self = art::Thread::FromManagedThread(soa, thread); - } InternalThreadState thread_state = {}; - art::MutexLock tscl_mu(soa.Self(), *art::Locks::thread_suspend_count_lock_); - thread_state.native_thread = self; - if (self == nullptr || self->IsStillStarting()) { + art::MutexLock tscl_mu(art::Thread::Current(), *art::Locks::thread_suspend_count_lock_); + thread_state.native_thread = target; + if (target == nullptr || target->IsStillStarting()) { thread_state.art_state = art::ThreadState::kStarting; thread_state.thread_user_code_suspend_count = 0; } else { - thread_state.art_state = self->GetState(); - thread_state.thread_user_code_suspend_count = self->GetUserCodeSuspendCount(); + thread_state.art_state = target->GetState(); + thread_state.thread_user_code_suspend_count = target->GetUserCodeSuspendCount(); } return thread_state; } @@ -456,7 +471,12 @@ jvmtiError ThreadUtil::GetThreadState(jvmtiEnv* env ATTRIBUTE_UNUSED, } art::ScopedObjectAccess soa(self); art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_); - state = GetNativeThreadState(thread, soa); + jvmtiError err = ERR(INTERNAL); + art::Thread* target = nullptr; + if (!GetNativeThread(thread, soa, &target, &err)) { + return err; + } + state = GetNativeThreadState(target); if (state.art_state == art::ThreadState::kStarting) { break; } @@ -578,12 +598,10 @@ jvmtiError ThreadUtil::SetThreadLocalStorage(jvmtiEnv* env, jthread thread, cons art::Thread* self = art::Thread::Current(); art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); - } - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } JvmtiGlobalTLSData* global_tls = reinterpret_cast<JvmtiGlobalTLSData*>(target->GetCustomTLS()); @@ -607,12 +625,10 @@ jvmtiError ThreadUtil::GetThreadLocalStorage(jvmtiEnv* env, art::Thread* self = art::Thread::Current(); art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = GetNativeThread(thread, soa); - if (target == nullptr && thread == nullptr) { - return ERR(INVALID_THREAD); - } - if (target == nullptr) { - return ERR(THREAD_NOT_ALIVE); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } JvmtiGlobalTLSData* global_tls = reinterpret_cast<JvmtiGlobalTLSData*>(target->GetCustomTLS()); @@ -731,9 +747,13 @@ jvmtiError ThreadUtil::SuspendOther(art::Thread* self, { art::ScopedObjectAccess soa(self); art::MutexLock thread_list_mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = GetNativeThread(target_jthread, soa); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!GetAliveNativeThread(target_jthread, soa, &target, &err)) { + return err; + } art::ThreadState state = target->GetState(); - if (state == art::ThreadState::kTerminated || state == art::ThreadState::kStarting) { + if (state == art::ThreadState::kStarting || target->IsStillStarting()) { return ERR(THREAD_NOT_ALIVE); } else { art::MutexLock thread_suspend_count_mu(self, *art::Locks::thread_suspend_count_lock_); @@ -789,9 +809,10 @@ jvmtiError ThreadUtil::SuspendThread(jvmtiEnv* env ATTRIBUTE_UNUSED, jthread thr { art::ScopedObjectAccess soa(self); art::MutexLock mu(self, *art::Locks::thread_list_lock_); - art::Thread* target = GetNativeThread(thread, soa); - if (target == nullptr) { - return ERR(INVALID_THREAD); + art::Thread* target = nullptr; + jvmtiError err = ERR(INTERNAL); + if (!GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } else if (target == self) { target_is_self = true; } @@ -825,16 +846,14 @@ jvmtiError ThreadUtil::ResumeThread(jvmtiEnv* env ATTRIBUTE_UNUSED, // have the 'suspend_lock' locked here. art::ScopedObjectAccess soa(self); art::MutexLock tll_mu(self, *art::Locks::thread_list_lock_); - target = GetNativeThread(thread, soa); - if (target == nullptr) { - return ERR(INVALID_THREAD); + jvmtiError err = ERR(INTERNAL); + if (!GetAliveNativeThread(thread, soa, &target, &err)) { + return err; } 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); - } else if (target->GetState() == art::ThreadState::kTerminated) { - return ERR(THREAD_NOT_ALIVE); } // The JVMTI spec requires us to return THREAD_NOT_SUSPENDED if it is alive but we really // cannot tell why resume failed. @@ -859,6 +878,22 @@ jvmtiError ThreadUtil::ResumeThread(jvmtiEnv* env ATTRIBUTE_UNUSED, } while (true); } +static bool IsCurrentThread(jthread thr) { + if (thr == nullptr) { + return true; + } + art::Thread* self = art::Thread::Current(); + art::ScopedObjectAccess soa(self); + art::MutexLock mu(self, *art::Locks::thread_list_lock_); + art::Thread* target = nullptr; + jvmtiError err_unused = ERR(INTERNAL); + if (ThreadUtil::GetNativeThread(thr, soa, &target, &err_unused)) { + return target == self; + } else { + return false; + } +} + // 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 @@ -878,17 +913,12 @@ jvmtiError ThreadUtil::SuspendThreadList(jvmtiEnv* env, // 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); - art::MutexLock mu(self, *art::Locks::thread_list_lock_); - if (threads[i] == nullptr || GetNativeThread(threads[i], soa) == self) { - current_thread_indexes.push_back(i); - continue; - } + if (IsCurrentThread(threads[i])) { + current_thread_indexes.push_back(i); + } else { + results[i] = env->SuspendThread(threads[i]); } - results[i] = env->SuspendThread(threads[i]); } if (!current_thread_indexes.empty()) { jint first_current_thread_index = current_thread_indexes[0]; diff --git a/openjdkjvmti/ti_thread.h b/openjdkjvmti/ti_thread.h index 57b194362f..ceebff67ec 100644 --- a/openjdkjvmti/ti_thread.h +++ b/openjdkjvmti/ti_thread.h @@ -93,8 +93,24 @@ class ThreadUtil { const jthread* threads, jvmtiError* results); - static art::Thread* GetNativeThread(jthread thread, - const art::ScopedObjectAccessAlreadyRunnable& soa) + // Returns true if we decoded the thread and it is alive, false otherwise with an appropriate + // error placed into 'err'. A thread is alive if it has had it's 'start' function called and has + // (or at least could have) executed managed code and has not yet returned past it's first managed + // frame. This means that the thread returned might have IsStillStarting() return true. Code that + // does not consider that alive should check manually. + static bool GetAliveNativeThread(jthread thread, + const art::ScopedObjectAccessAlreadyRunnable& soa, + /*out*/ art::Thread** thr, + /*out*/ jvmtiError* err) + REQUIRES_SHARED(art::Locks::mutator_lock_) + REQUIRES(art::Locks::thread_list_lock_); + + // Returns true if we decoded the thread, false otherwise with an appropriate error placed into + // 'err' + static bool GetNativeThread(jthread thread, + const art::ScopedObjectAccessAlreadyRunnable& soa, + /*out*/ art::Thread** thr, + /*out*/ jvmtiError* err) REQUIRES_SHARED(art::Locks::mutator_lock_) REQUIRES(art::Locks::thread_list_lock_); |