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
diff --git a/src/debugger.cc b/src/debugger.cc
index a25f5dc..aad75b1 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -1475,10 +1475,10 @@
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 @@
// 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::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::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 091ecd6..24aa730 100644
--- a/src/native/dalvik_system_VMStack.cc
+++ b/src/native/dalvik_system_VMStack.cc
@@ -25,12 +25,10 @@
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 adc246a..2a6f177 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 @@
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 be92068..66c27e4 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 @@
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 @@
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 8262f8a..9983c7d 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -572,7 +572,7 @@
"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 dfeb7f1..d43d2ef 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -115,7 +115,7 @@
// 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 @@
}
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 @@
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* 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 @@
}
}
- self->GetJniEnv()->locals.AssertEmpty();
return self;
}
@@ -326,14 +327,11 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
// 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 @@
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 @@
}
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 @@
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 @@
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 34dfdbd..a58fb65 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -283,7 +283,7 @@
// 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 @@
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 dee8148..5201928 100755
--- a/test/etc/host-run-test-jar
+++ b/test/etc/host-run-test-jar
@@ -92,7 +92,7 @@
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 08df02b..47d8c32 100755
--- a/test/etc/push-and-run-test-jar
+++ b/test/etc/push-and-run-test-jar
@@ -114,7 +114,7 @@
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"
