diff options
-rw-r--r-- | runtime/debugger.cc | 9 | ||||
-rw-r--r-- | runtime/native/dalvik_system_VMStack.cc | 5 | ||||
-rw-r--r-- | runtime/native/java_lang_Thread.cc | 5 | ||||
-rw-r--r-- | runtime/thread_list.cc | 35 | ||||
-rw-r--r-- | runtime/thread_list.h | 4 | ||||
-rw-r--r-- | test/051-thread/expected.txt | 3 | ||||
-rw-r--r-- | test/051-thread/src/Main.java | 21 |
7 files changed, 64 insertions, 18 deletions
diff --git a/runtime/debugger.cc b/runtime/debugger.cc index ffd15f638a..9136f9f8d8 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -2281,8 +2281,9 @@ JDWP::JdwpError Dbg::SuspendThread(JDWP::ObjectId thread_id, bool request_suspen // trying to suspend this one. MutexLock mu(self, *Locks::thread_list_suspend_thread_lock_); bool timed_out; - Thread* thread = ThreadList::SuspendThreadByPeer(peer.get(), request_suspension, true, - &timed_out); + ThreadList* thread_list = Runtime::Current()->GetThreadList(); + Thread* thread = thread_list->SuspendThreadByPeer(peer.get(), request_suspension, true, + &timed_out); if (thread != NULL) { return JDWP::ERR_NONE; } else if (timed_out) { @@ -3171,8 +3172,8 @@ class ScopedThreadSuspension { { // Take suspend thread lock to avoid races with threads trying to suspend this one. MutexLock mu(soa.Self(), *Locks::thread_list_suspend_thread_lock_); - suspended_thread = ThreadList::SuspendThreadByPeer(thread_peer, true, true, - &timed_out); + ThreadList* thread_list = Runtime::Current()->GetThreadList(); + suspended_thread = thread_list->SuspendThreadByPeer(thread_peer, true, true, &timed_out); } CHECK_EQ(soa.Self()->TransitionFromSuspendedToRunnable(), kWaitingForDebuggerSuspension); if (suspended_thread == nullptr) { diff --git a/runtime/native/dalvik_system_VMStack.cc b/runtime/native/dalvik_system_VMStack.cc index b0792293e0..eef1c46c1f 100644 --- a/runtime/native/dalvik_system_VMStack.cc +++ b/runtime/native/dalvik_system_VMStack.cc @@ -34,12 +34,13 @@ static jobject GetThreadStack(const ScopedFastNativeObjectAccess& soa, jobject p } else { // Suspend thread to build stack trace. soa.Self()->TransitionFromRunnableToSuspended(kNative); + ThreadList* thread_list = Runtime::Current()->GetThreadList(); bool timed_out; Thread* thread; { // Take suspend thread lock to avoid races with threads trying to suspend this one. MutexLock mu(soa.Self(), *Locks::thread_list_suspend_thread_lock_); - thread = ThreadList::SuspendThreadByPeer(peer, true, false, &timed_out); + thread = thread_list->SuspendThreadByPeer(peer, true, false, &timed_out); } if (thread != nullptr) { // Must be runnable to create returned array. @@ -47,7 +48,7 @@ static jobject GetThreadStack(const ScopedFastNativeObjectAccess& soa, jobject p trace = thread->CreateInternalStackTrace<false>(soa); soa.Self()->TransitionFromRunnableToSuspended(kNative); // Restart suspended thread. - Runtime::Current()->GetThreadList()->Resume(thread, false); + thread_list->Resume(thread, false); } else { if (timed_out) { LOG(ERROR) << "Trying to get thread's stack failed as the thread failed to suspend within a " diff --git a/runtime/native/java_lang_Thread.cc b/runtime/native/java_lang_Thread.cc index 8f83f96318..c0c72651de 100644 --- a/runtime/native/java_lang_Thread.cc +++ b/runtime/native/java_lang_Thread.cc @@ -128,19 +128,20 @@ static void Thread_nativeSetName(JNIEnv* env, jobject peer, jstring java_name) { // Suspend thread to avoid it from killing itself while we set its name. We don't just hold the // thread list lock to avoid this, as setting the thread name causes mutator to lock/unlock // in the DDMS send code. + ThreadList* thread_list = Runtime::Current()->GetThreadList(); bool timed_out; // Take suspend thread lock to avoid races with threads trying to suspend this one. Thread* thread; { MutexLock mu(self, *Locks::thread_list_suspend_thread_lock_); - thread = ThreadList::SuspendThreadByPeer(peer, true, false, &timed_out); + thread = thread_list->SuspendThreadByPeer(peer, true, false, &timed_out); } if (thread != NULL) { { ScopedObjectAccess soa(env); thread->SetThreadName(name.c_str()); } - Runtime::Current()->GetThreadList()->Resume(thread, false); + thread_list->Resume(thread, false); } 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/thread_list.cc b/runtime/thread_list.cc index 5077a89ee9..740e3b086f 100644 --- a/runtime/thread_list.cc +++ b/runtime/thread_list.cc @@ -406,7 +406,8 @@ void ThreadList::ResumeAll() { void ThreadList::Resume(Thread* thread, bool for_debugger) { Thread* self = Thread::Current(); DCHECK_NE(thread, self); - VLOG(threads) << "Resume(" << *thread << ") starting..." << (for_debugger ? " (debugger)" : ""); + VLOG(threads) << "Resume(" << reinterpret_cast<void*>(thread) << ") starting..." + << (for_debugger ? " (debugger)" : ""); { // To check Contains. @@ -415,18 +416,22 @@ void ThreadList::Resume(Thread* thread, bool for_debugger) { MutexLock mu2(self, *Locks::thread_suspend_count_lock_); DCHECK(thread->IsSuspended()); 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; } thread->ModifySuspendCount(self, -1, for_debugger); } { - VLOG(threads) << "Resume(" << *thread << ") waking others"; + VLOG(threads) << "Resume(" << reinterpret_cast<void*>(thread) << ") waking others"; MutexLock mu(self, *Locks::thread_suspend_count_lock_); Thread::resume_cond_->Broadcast(self); } - VLOG(threads) << "Resume(" << *thread << ") complete"; + VLOG(threads) << "Resume(" << reinterpret_cast<void*>(thread) << ") complete"; } static void ThreadSuspendByPeerWarning(Thread* self, int level, const char* message, jobject peer) { @@ -451,6 +456,7 @@ Thread* ThreadList::SuspendThreadByPeer(jobject peer, bool request_suspension, bool did_suspend_request = false; *timed_out = false; Thread* self = Thread::Current(); + VLOG(threads) << "SuspendThreadByPeer starting"; while (true) { Thread* thread; { @@ -462,10 +468,16 @@ Thread* ThreadList::SuspendThreadByPeer(jobject peer, bool request_suspension, ScopedObjectAccess soa(self); MutexLock mu(self, *Locks::thread_list_lock_); thread = Thread::FromManagedThread(soa, peer); - if (thread == NULL) { + if (thread == nullptr) { ThreadSuspendByPeerWarning(self, WARNING, "No such thread for suspend", peer); - return NULL; + return nullptr; + } + if (!Contains(thread)) { + VLOG(threads) << "SuspendThreadByPeer failed for unattached thread: " + << reinterpret_cast<void*>(thread); + return nullptr; } + VLOG(threads) << "SuspendThreadByPeer found thread: " << *thread; { MutexLock mu(self, *Locks::thread_suspend_count_lock_); if (request_suspension) { @@ -485,6 +497,7 @@ Thread* ThreadList::SuspendThreadByPeer(jobject peer, bool request_suspension, // count, or else we've waited and it has self suspended) or is the current thread, we're // done. if (thread->IsSuspended()) { + VLOG(threads) << "SuspendThreadByPeer thread suspended: " << *thread; return thread; } if (total_delay_us >= kTimeoutUs) { @@ -493,11 +506,12 @@ Thread* ThreadList::SuspendThreadByPeer(jobject peer, bool request_suspension, thread->ModifySuspendCount(soa.Self(), -1, debug_suspension); } *timed_out = true; - return NULL; + return nullptr; } } // Release locks and come out of runnable state. } + VLOG(threads) << "SuspendThreadByPeer sleeping to allow thread chance to suspend"; ThreadSuspendSleep(self, &delay_us, &total_delay_us); } } @@ -515,6 +529,7 @@ Thread* ThreadList::SuspendThreadByThreadId(uint32_t thread_id, bool debug_suspe Thread* suspended_thread = nullptr; Thread* self = Thread::Current(); CHECK_NE(thread_id, kInvalidThreadId); + VLOG(threads) << "SuspendThreadByThreadId starting"; while (true) { { // Note: this will transition to runnable and potentially suspend. We ensure only one thread @@ -536,8 +551,10 @@ Thread* ThreadList::SuspendThreadByThreadId(uint32_t thread_id, bool debug_suspe << " no longer in thread list"; // There's a race in inflating a lock and the owner giving up ownership and then dying. ThreadSuspendByThreadIdWarning(WARNING, "No such thread id for suspend", thread_id); - return NULL; + return nullptr; } + VLOG(threads) << "SuspendThreadByThreadId found thread: " << *thread; + DCHECK(Contains(thread)); { MutexLock mu(self, *Locks::thread_suspend_count_lock_); if (suspended_thread == nullptr) { @@ -557,6 +574,7 @@ Thread* ThreadList::SuspendThreadByThreadId(uint32_t thread_id, bool debug_suspe // count, or else we've waited and it has self suspended) or is the current thread, we're // done. if (thread->IsSuspended()) { + VLOG(threads) << "SuspendThreadByThreadId thread suspended: " << *thread; return thread; } if (total_delay_us >= kTimeoutUs) { @@ -565,11 +583,12 @@ Thread* ThreadList::SuspendThreadByThreadId(uint32_t thread_id, bool debug_suspe thread->ModifySuspendCount(soa.Self(), -1, debug_suspension); } *timed_out = true; - return NULL; + return nullptr; } } // Release locks and come out of runnable state. } + VLOG(threads) << "SuspendThreadByThreadId sleeping to allow thread chance to suspend"; ThreadSuspendSleep(self, &delay_us, &total_delay_us); } } diff --git a/runtime/thread_list.h b/runtime/thread_list.h index 1b67ac0588..bb4f7750f5 100644 --- a/runtime/thread_list.h +++ b/runtime/thread_list.h @@ -66,8 +66,8 @@ class ThreadList { // If the thread should be suspended then value of request_suspension should be true otherwise // the routine will wait for a previous suspend request. If the suspension times out then *timeout // is set to true. - static Thread* SuspendThreadByPeer(jobject peer, bool request_suspension, bool debug_suspension, - bool* timed_out) + Thread* SuspendThreadByPeer(jobject peer, bool request_suspension, bool debug_suspension, + bool* timed_out) EXCLUSIVE_LOCKS_REQUIRED(Locks::thread_list_suspend_thread_lock_) LOCKS_EXCLUDED(Locks::mutator_lock_, Locks::thread_list_lock_, diff --git a/test/051-thread/expected.txt b/test/051-thread/expected.txt index 7139b7f027..943d1dfac2 100644 --- a/test/051-thread/expected.txt +++ b/test/051-thread/expected.txt @@ -6,4 +6,7 @@ testThreadDaemons @ Got expected setDaemon exception testThreadDaemons @ Thread bailing testThreadDaemons finished testSleepZero finished +testSetName starting +testSetName running +testSetName finished thread test done diff --git a/test/051-thread/src/Main.java b/test/051-thread/src/Main.java index 608b7e0878..390685d049 100644 --- a/test/051-thread/src/Main.java +++ b/test/051-thread/src/Main.java @@ -25,6 +25,7 @@ public class Main { testThreadCapacity(); testThreadDaemons(); testSleepZero(); + testSetName(); System.out.println("thread test done"); } @@ -112,4 +113,24 @@ public class Main { } System.out.print("testSleepZero finished\n"); } + + private static void testSetName() throws Exception { + System.out.print("testSetName starting\n"); + Thread thread = new Thread() { + @Override + public void run() { + System.out.print("testSetName running\n"); + } + }; + thread.start(); + thread.setName("HelloWorld"); // b/17302037 hang if setName called after start + if (!thread.getName().equals("HelloWorld")) { + throw new AssertionError("Unexpected thread name: " + thread.getName()); + } + thread.join(); + if (!thread.getName().equals("HelloWorld")) { + throw new AssertionError("Unexpected thread name after join: " + thread.getName()); + } + System.out.print("testSetName finished\n"); + } } |