diff options
author | 2012-09-25 16:54:50 -0700 | |
---|---|---|
committer | 2012-09-25 22:36:20 -0700 | |
commit | dbe6f4613ae0161b169f4fca8a616b0b393370ab (patch) | |
tree | bcf21fa802f8ae261522e8c9bab3685020908d13 | |
parent | 474b6da273c7ce6df50a4e51eb9929a77e1611c3 (diff) |
Change Thread::peer_ to be a jobject instead of an Object*
Fixes issue where GC was freeing the java peer if the parent thread exited before the child thread got registered.
Change-Id: I6fbe74865d5827d243ac55fc396679afa97ff2f9
-rw-r--r-- | src/debugger.cc | 16 | ||||
-rw-r--r-- | src/native/dalvik_system_VMStack.cc | 8 | ||||
-rw-r--r-- | src/native/java_lang_Thread.cc | 8 | ||||
-rw-r--r-- | src/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc | 5 | ||||
-rw-r--r-- | src/runtime.cc | 2 | ||||
-rw-r--r-- | src/thread.cc | 97 | ||||
-rw-r--r-- | src/thread.h | 4 | ||||
-rwxr-xr-x | test/etc/host-run-test-jar | 2 | ||||
-rwxr-xr-x | test/etc/push-and-run-test-jar | 2 |
9 files changed, 72 insertions, 72 deletions
diff --git a/src/debugger.cc b/src/debugger.cc index a25f5dc875..aad75b1493 100644 --- a/src/debugger.cc +++ b/src/debugger.cc @@ -1475,10 +1475,10 @@ bool Dbg::IsSuspended(JDWP::ObjectId threadId) { void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId>& thread_ids) { class ThreadListVisitor { public: - ThreadListVisitor(const ScopedObjectAccessUnchecked& ts, Object* thread_group, + ThreadListVisitor(const ScopedObjectAccessUnchecked& soa, Object* thread_group, std::vector<JDWP::ObjectId>& thread_ids) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) - : ts_(ts), thread_group_(thread_group), thread_ids_(thread_ids) {} + : soa_(soa), thread_group_(thread_group), thread_ids_(thread_ids) {} static void Visit(Thread* t, void* arg) { reinterpret_cast<ThreadListVisitor*>(arg)->Visit(t); @@ -1492,13 +1492,13 @@ void Dbg::GetThreads(JDWP::ObjectId thread_group_id, std::vector<JDWP::ObjectId> // query all threads, so it's easier if we just don't tell them about this thread. return; } - if (thread_group_ == NULL || t->GetThreadGroup(ts_) == thread_group_) { - thread_ids_.push_back(gRegistry->Add(t->GetPeer())); + if (thread_group_ == NULL || t->GetThreadGroup(soa_) == thread_group_) { + thread_ids_.push_back(gRegistry->Add(soa_.Decode<Object*>(t->GetPeer()))); } } private: - const ScopedObjectAccessUnchecked& ts_; + const ScopedObjectAccessUnchecked& soa_; Object* const thread_group_; std::vector<JDWP::ObjectId>& thread_ids_; }; @@ -1607,7 +1607,8 @@ JDWP::JdwpError Dbg::GetThreadFrames(JDWP::ObjectId thread_id, size_t start_fram } JDWP::ObjectId Dbg::GetThreadSelfId() { - return gRegistry->Add(Thread::Current()->GetPeer()); + ScopedObjectAccessUnchecked soa(Thread::Current()); + return gRegistry->Add(soa.Decode<Object*>(Thread::Current()->GetPeer())); } void Dbg::SuspendVM() { @@ -2708,7 +2709,8 @@ void Dbg::DdmSetThreadNotification(bool enable) { void Dbg::PostThreadStartOrStop(Thread* t, uint32_t type) { if (IsDebuggerActive()) { - JDWP::ObjectId id = gRegistry->Add(t->GetPeer()); + ScopedObjectAccessUnchecked soa(Thread::Current()); + JDWP::ObjectId id = gRegistry->Add(soa.Decode<Object*>(t->GetPeer())); gJdwpState->PostThreadChange(id, type == CHUNK_TYPE("THCR")); // If this thread's just joined the party while we're already debugging, make sure it knows // to give us updates when it's running. diff --git a/src/native/dalvik_system_VMStack.cc b/src/native/dalvik_system_VMStack.cc index 091ecd61ac..24aa730cdc 100644 --- a/src/native/dalvik_system_VMStack.cc +++ b/src/native/dalvik_system_VMStack.cc @@ -25,12 +25,10 @@ namespace art { static jobject GetThreadStack(JNIEnv* env, jobject peer) { bool timeout; - { + Thread* self = Thread::Current(); + if (env->IsSameObject(peer, self->GetPeer())) { ScopedObjectAccess soa(env); - Thread* self = Thread::Current(); - if (soa.Decode<Object*>(peer) == self->GetPeer()) { - return self->CreateInternalStackTrace(soa); - } + return self->CreateInternalStackTrace(soa); } // Suspend thread to build stack trace. Thread* thread = Thread::SuspendForDebugger(peer, true, &timeout); diff --git a/src/native/java_lang_Thread.cc b/src/native/java_lang_Thread.cc index adc246aa28..2a6f177ce5 100644 --- a/src/native/java_lang_Thread.cc +++ b/src/native/java_lang_Thread.cc @@ -25,8 +25,7 @@ namespace art { static jobject Thread_currentThread(JNIEnv* env, jclass) { - ScopedObjectAccess soa(env); - return soa.AddLocalReference<jobject>(soa.Self()->GetPeer()); + return reinterpret_cast<JNIEnvExt*>(env)->self->GetPeer(); } static jboolean Thread_interrupted(JNIEnv* env, jclass) { @@ -110,9 +109,8 @@ static void Thread_nativeSetName(JNIEnv* env, jobject peer, jstring java_name) { ScopedUtfChars name(env, java_name); { ScopedObjectAccess soa(env); - Thread* self = Thread::Current(); - if (soa.Decode<Object*>(peer) == self->GetPeer()) { - self->SetThreadName(name.c_str()); + if (soa.Env()->IsSameObject(peer, soa.Self()->GetPeer())) { + soa.Self()->SetThreadName(name.c_str()); return; } } diff --git a/src/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc b/src/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc index be92068a58..66c27e434f 100644 --- a/src/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc +++ b/src/native/org_apache_harmony_dalvik_ddmc_DdmVmInternal.cc @@ -39,7 +39,7 @@ static jboolean DdmVmInternal_getRecentAllocationStatus(JNIEnv*, jclass) { return Dbg::IsAllocTrackingEnabled(); } -static jobject FindThreadByThinLockId(JNIEnv* env, uint32_t thin_lock_id) { +static jobject FindThreadByThinLockId(JNIEnv*, uint32_t thin_lock_id) { struct ThreadFinder { explicit ThreadFinder(uint32_t thin_lock_id) : thin_lock_id(thin_lock_id), thread(NULL) { } @@ -60,8 +60,7 @@ static jobject FindThreadByThinLockId(JNIEnv* env, uint32_t thin_lock_id) { Runtime::Current()->GetThreadList()->ForEach(ThreadFinder::Callback, &finder); } if (finder.thread != NULL) { - ScopedObjectAccess soa(env); - return soa.AddLocalReference<jobject>(finder.thread->GetPeer()); + return finder.thread->GetPeer(); } else { return NULL; } diff --git a/src/runtime.cc b/src/runtime.cc index 8262f8ae46..9983c7d40e 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -572,7 +572,7 @@ static void CreateSystemClassLoader() { "Ljava/lang/ClassLoader;"); CHECK(contextClassLoader != NULL); - contextClassLoader->SetObject(soa.Self()->GetPeer(), class_loader); + contextClassLoader->SetObject(soa.Decode<Object*>(soa.Self()->GetPeer()), class_loader); } void Runtime::Start() { diff --git a/src/thread.cc b/src/thread.cc index dfeb7f134b..d43d2ef617 100644 --- a/src/thread.cc +++ b/src/thread.cc @@ -115,7 +115,7 @@ void* Thread::CreateCallback(void* arg) { // Invoke the 'run' method of our java.lang.Thread. CHECK(self->peer_ != NULL); - Object* receiver = self->peer_; + Object* receiver = soa.Decode<Object*>(self->peer_); jmethodID mid = WellKnownClasses::java_lang_Thread_run; AbstractMethod* m = receiver->GetClass()->FindVirtualMethodForVirtualOrInterface(soa.DecodeMethod(mid)); m->Invoke(self, receiver, NULL, NULL); @@ -214,17 +214,19 @@ static void TearDownAlternateSignalStack() { } void Thread::CreateNativeThread(JNIEnv* env, jobject java_peer, size_t stack_size, bool daemon) { + CHECK(java_peer != NULL); + Thread* native_thread = new Thread(daemon); { ScopedObjectAccess soa(env); - Object* peer = soa.Decode<Object*>(java_peer); - CHECK(peer != NULL); - native_thread->peer_ = peer; - + // Use global JNI ref to hold peer live whilst child thread starts. + native_thread->peer_ = env->NewGlobalRef(java_peer); stack_size = FixStackSize(stack_size); // Thread.start is synchronized, so we know that vmData is 0, and know that we're not racing to // assign it. + Object* peer = soa.Decode<Object*>(native_thread->peer_); + CHECK(peer != NULL); SetVmData(soa, peer, native_thread); } @@ -247,6 +249,9 @@ void Thread::CreateNativeThread(JNIEnv* env, jobject java_peer, size_t stack_siz PrettySize(stack_size).c_str(), strerror(pthread_create_result))); Thread::Current()->ThrowOutOfMemoryError(msg.c_str()); } + // If we failed, manually delete the global reference since Thread::Init will not have been run. + env->DeleteGlobalRef(native_thread->peer_); + native_thread->peer_ = NULL; delete native_thread; return; } @@ -289,11 +294,8 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, jobject thread_g Thread* self = new Thread(as_daemon); self->Init(); - { - MutexLock mu(*Locks::thread_suspend_count_lock_); - CHECK_NE(self->GetState(), kRunnable); - self->SetState(kNative); - } + CHECK_NE(self->GetState(), kRunnable); + self->SetState(kNative); // If we're the main thread, ClassLinker won't be created until after we're attached, // so that thread needs a two-stage attach. Regular threads don't need this hack. @@ -309,7 +311,6 @@ Thread* Thread::Attach(const char* thread_name, bool as_daemon, jobject thread_g } } - self->GetJniEnv()->locals.AssertEmpty(); return self; } @@ -326,14 +327,11 @@ void Thread::CreatePeer(const char* name, bool as_daemon, jobject thread_group) jboolean thread_is_daemon = as_daemon; ScopedLocalRef<jobject> peer(env, env->AllocObject(WellKnownClasses::java_lang_Thread)); - { - ScopedObjectAccess soa(env); - peer_ = DecodeJObject(peer.get()); - if (peer_ == NULL) { - CHECK(IsExceptionPending()); - return; - } + if (peer.get() == NULL) { + CHECK(IsExceptionPending()); + return; } + peer_ = env->NewGlobalRef(peer.get()); env->CallNonvirtualVoidMethod(peer.get(), WellKnownClasses::java_lang_Thread, WellKnownClasses::java_lang_Thread_init, @@ -341,17 +339,22 @@ void Thread::CreatePeer(const char* name, bool as_daemon, jobject thread_group) AssertNoPendingException(); ScopedObjectAccess soa(this); - SetVmData(soa, peer_, Thread::Current()); + Object* native_peer = soa.Decode<Object*>(peer.get()); + SetVmData(soa, native_peer, Thread::Current()); SirtRef<String> peer_thread_name(GetThreadName(soa)); if (peer_thread_name.get() == NULL) { // The Thread constructor should have set the Thread.name to a // non-null value. However, because we can run without code // available (in the compiler, in tests), we manually assign the // fields the constructor should have set. - soa.DecodeField(WellKnownClasses::java_lang_Thread_daemon)->SetBoolean(peer_, thread_is_daemon); - soa.DecodeField(WellKnownClasses::java_lang_Thread_group)->SetObject(peer_, soa.Decode<Object*>(thread_group)); - soa.DecodeField(WellKnownClasses::java_lang_Thread_name)->SetObject(peer_, soa.Decode<Object*>(thread_name.get())); - soa.DecodeField(WellKnownClasses::java_lang_Thread_priority)->SetInt(peer_, thread_priority); + soa.DecodeField(WellKnownClasses::java_lang_Thread_daemon)-> + SetBoolean(native_peer, thread_is_daemon); + soa.DecodeField(WellKnownClasses::java_lang_Thread_group)-> + SetObject(native_peer, soa.Decode<Object*>(thread_group)); + soa.DecodeField(WellKnownClasses::java_lang_Thread_name)-> + SetObject(native_peer, soa.Decode<Object*>(thread_name.get())); + soa.DecodeField(WellKnownClasses::java_lang_Thread_priority)-> + SetInt(native_peer, thread_priority); peer_thread_name.reset(GetThreadName(soa)); } // 'thread_name' may have been null, so don't trust 'peer_thread_name' to be non-null. @@ -440,7 +443,8 @@ void Thread::Dump(std::ostream& os) const { String* Thread::GetThreadName(const ScopedObjectAccessUnchecked& soa) const { Field* f = soa.DecodeField(WellKnownClasses::java_lang_Thread_name); - return (peer_ != NULL) ? reinterpret_cast<String*>(f->GetObject(peer_)) : NULL; + Object* native_peer = soa.Decode<Object*>(peer_); + return (peer_ != NULL) ? reinterpret_cast<String*>(f->GetObject(native_peer)) : NULL; } void Thread::GetThreadName(std::string& name) const { @@ -639,8 +643,9 @@ void Thread::DumpState(std::ostream& os, const Thread* thread, pid_t tid) { if (thread != NULL && thread->peer_ != NULL) { ScopedObjectAccess soa(Thread::Current()); - priority = soa.DecodeField(WellKnownClasses::java_lang_Thread_priority)->GetInt(thread->peer_); - is_daemon = soa.DecodeField(WellKnownClasses::java_lang_Thread_daemon)->GetBoolean(thread->peer_); + Object* native_peer = soa.Decode<Object*>(thread->peer_); + priority = soa.DecodeField(WellKnownClasses::java_lang_Thread_priority)->GetInt(native_peer); + is_daemon = soa.DecodeField(WellKnownClasses::java_lang_Thread_daemon)->GetBoolean(native_peer); Object* thread_group = thread->GetThreadGroup(soa); if (thread_group != NULL) { @@ -795,12 +800,7 @@ struct StackDumpVisitor : public StackVisitor { void Thread::DumpStack(std::ostream& os) const { // If we're currently in native code, dump that stack before dumping the managed stack. - ThreadState state; - { - MutexLock mu(*Locks::thread_suspend_count_lock_); - state = GetState(); - } - if (state == kNative) { + if (GetState() == kNative) { DumpKernelStack(os, GetTid(), " kernel: ", false); DumpNativeStack(os, GetTid(), " native: ", false); } @@ -935,13 +935,14 @@ void Thread::Destroy() { RemoveFromThreadGroup(soa); // this.vmData = 0; - SetVmData(soa, peer_, NULL); + SetVmData(soa, soa.Decode<Object*>(peer_), NULL); Dbg::PostThreadDeath(self); // Thread.join() is implemented as an Object.wait() on the Thread.lock // object. Signal anyone who is waiting. - Object* lock = soa.DecodeField(WellKnownClasses::java_lang_Thread_lock)->GetObject(peer_); + Object* lock = soa.DecodeField(WellKnownClasses::java_lang_Thread_lock)-> + GetObject(soa.Decode<Object*>(peer_)); // (This conditional is only needed for tests, where Thread.lock won't have been set.) if (lock != NULL) { lock->MonitorEnter(self); @@ -952,15 +953,18 @@ void Thread::Destroy() { } Thread::~Thread() { + if (jni_env_ != NULL && peer_ != NULL) { + // If pthread_create fails we don't have a jni env here. + jni_env_->DeleteGlobalRef(peer_); + } + peer_ = NULL; + delete jni_env_; jni_env_ = NULL; - { - MutexLock mu(*Locks::thread_suspend_count_lock_); - CHECK_NE(GetState(), kRunnable); - // We may be deleting a still born thread. - SetStateUnsafe(kTerminated); - } + CHECK_NE(GetState(), kRunnable); + // We may be deleting a still born thread. + SetStateUnsafe(kTerminated); delete wait_cond_; delete wait_mutex_; @@ -986,7 +990,8 @@ void Thread::HandleUncaughtExceptions(const ScopedObjectAccess& soa) { // If the thread has its own handler, use that. Object* handler = - soa.DecodeField(WellKnownClasses::java_lang_Thread_uncaughtHandler)->GetObject(peer_); + soa.DecodeField(WellKnownClasses::java_lang_Thread_uncaughtHandler)-> + GetObject(soa.Decode<Object*>(peer_)); if (handler == NULL) { // Otherwise use the thread group's default handler. handler = GetThreadGroup(soa); @@ -996,7 +1001,7 @@ void Thread::HandleUncaughtExceptions(const ScopedObjectAccess& soa) { jmethodID mid = WellKnownClasses::java_lang_Thread$UncaughtExceptionHandler_uncaughtException; AbstractMethod* m = handler->GetClass()->FindVirtualMethodForVirtualOrInterface(soa.DecodeMethod(mid)); JValue args[2]; - args[0].SetL(peer_); + args[0].SetL(soa.Decode<Object*>(peer_)); args[1].SetL(exception); m->Invoke(this, handler, args, NULL); @@ -1005,7 +1010,8 @@ void Thread::HandleUncaughtExceptions(const ScopedObjectAccess& soa) { } Object* Thread::GetThreadGroup(const ScopedObjectAccessUnchecked& soa) const { - return soa.DecodeField(WellKnownClasses::java_lang_Thread_group)->GetObject(peer_); + return soa.DecodeField(WellKnownClasses::java_lang_Thread_group)-> + GetObject(soa.Decode<Object*>(peer_)); } void Thread::RemoveFromThreadGroup(const ScopedObjectAccess& soa) { @@ -1016,7 +1022,7 @@ void Thread::RemoveFromThreadGroup(const ScopedObjectAccess& soa) { jmethodID mid = WellKnownClasses::java_lang_ThreadGroup_removeThread; AbstractMethod* m = group->GetClass()->FindVirtualMethodForVirtualOrInterface(soa.DecodeMethod(mid)); JValue args[1]; - args[0].SetL(peer_); + args[0].SetL(soa.Decode<Object*>(peer_)); m->Invoke(this, group, args, NULL); } } @@ -1814,9 +1820,6 @@ void Thread::VisitRoots(Heap::RootVisitor* visitor, void* arg) { if (exception_ != NULL) { visitor(exception_, arg); } - if (peer_ != NULL) { - visitor(peer_, arg); - } if (class_loader_override_ != NULL) { visitor(class_loader_override_, arg); } diff --git a/src/thread.h b/src/thread.h index 34dfdbd639..a58fb65083 100644 --- a/src/thread.h +++ b/src/thread.h @@ -283,7 +283,7 @@ class PACKED Thread { // Sets the thread's name. void SetThreadName(const char* name) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - Object* GetPeer() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + jobject GetPeer() const { return peer_; } @@ -702,7 +702,7 @@ class PACKED Thread { Thread* self_; // Our managed peer (an instance of java.lang.Thread). - Object* peer_; + jobject peer_; // The "lowest addressable byte" of the stack byte* stack_begin_; diff --git a/test/etc/host-run-test-jar b/test/etc/host-run-test-jar index dee81489de..5201928549 100755 --- a/test/etc/host-run-test-jar +++ b/test/etc/host-run-test-jar @@ -92,7 +92,7 @@ if [ "$GDB" = "y" ]; then gdbargs="--args $exe" fi -JNI_OPTS="-Xjnigreflimit:256 -Xcheck:jni" +JNI_OPTS="-Xjnigreflimit:512 -Xcheck:jni" cd $ANDROID_BUILD_TOP $INVOKE_WITH $gdb $exe $gdbargs -Ximage:$ANDROID_ROOT/framework/core.art \ diff --git a/test/etc/push-and-run-test-jar b/test/etc/push-and-run-test-jar index 08df02b451..47d8c322a9 100755 --- a/test/etc/push-and-run-test-jar +++ b/test/etc/push-and-run-test-jar @@ -114,7 +114,7 @@ if [ "$DEBUG" = "y" ]; then DEBUG_OPTS="-agentlib:jdwp=transport=dt_socket,address=$PORT,server=y,suspend=y" fi -JNI_OPTS="-Xjnigreflimit:256 -Xcheck:jni" +JNI_OPTS="-Xjnigreflimit:512 -Xcheck:jni" cmdline="cd $DEX_LOCATION && mkdir art-cache && export ANDROID_DATA=$DEX_LOCATION && export DEX_LOCATION=$DEX_LOCATION && \ $INVOKE_WITH $OATEXEC $ZYGOTE $JNI_OPTS $DEBUG_OPTS -Ximage:/data/art-test/core.art -cp $DEX_LOCATION/$TEST_NAME.jar Main" |