Merge "JDWP: fix thread_list deadlock"
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 12fe863..c074b54 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -415,9 +415,8 @@
static Thread* DecodeThread(ScopedObjectAccessUnchecked& soa, JDWP::ObjectId thread_id,
JDWP::JdwpError* error)
- EXCLUSIVE_LOCKS_REQUIRED(Locks::thread_list_lock_)
- LOCKS_EXCLUDED(Locks::thread_suspend_count_lock_)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::thread_suspend_count_lock_) {
mirror::Object* thread_peer = Dbg::GetObjectRegistry()->Get<mirror::Object*>(thread_id, error);
if (thread_peer == nullptr) {
// This isn't even an object.
@@ -432,6 +431,7 @@
return nullptr;
}
+ MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
Thread* thread = Thread::FromManagedThread(soa, thread_peer);
// If thread is null then this a java.lang.Thread without a Thread*. Must be a un-started or a
// zombie.
@@ -856,17 +856,13 @@
};
ScopedObjectAccessUnchecked soa(Thread::Current());
- Thread* thread;
- {
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- JDWP::JdwpError error;
- thread = DecodeThread(soa, thread_id, &error);
- if (thread == nullptr) {
- return error;
- }
- if (!IsSuspendedForDebugger(soa, thread)) {
- return JDWP::ERR_THREAD_NOT_SUSPENDED;
- }
+ JDWP::JdwpError error;
+ Thread* thread = DecodeThread(soa, thread_id, &error);
+ if (thread == nullptr) {
+ return error;
+ }
+ if (!IsSuspendedForDebugger(soa, thread)) {
+ return JDWP::ERR_THREAD_NOT_SUSPENDED;
}
std::unique_ptr<Context> context(Context::Create());
OwnedMonitorVisitor visitor(thread, context.get(), monitors, stack_depths);
@@ -876,21 +872,17 @@
JDWP::JdwpError Dbg::GetContendedMonitor(JDWP::ObjectId thread_id,
JDWP::ObjectId* contended_monitor) {
- mirror::Object* contended_monitor_obj;
ScopedObjectAccessUnchecked soa(Thread::Current());
*contended_monitor = 0;
- {
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- JDWP::JdwpError error;
- Thread* thread = DecodeThread(soa, thread_id, &error);
- if (thread == nullptr) {
- return error;
- }
- if (!IsSuspendedForDebugger(soa, thread)) {
- return JDWP::ERR_THREAD_NOT_SUSPENDED;
- }
- contended_monitor_obj = Monitor::GetContendedMonitor(thread);
+ JDWP::JdwpError error;
+ Thread* thread = DecodeThread(soa, thread_id, &error);
+ if (thread == nullptr) {
+ return error;
}
+ if (!IsSuspendedForDebugger(soa, thread)) {
+ return JDWP::ERR_THREAD_NOT_SUSPENDED;
+ }
+ mirror::Object* contended_monitor_obj = Monitor::GetContendedMonitor(thread);
// Add() requires the thread_list_lock_ not held to avoid the lock
// level violation.
*contended_monitor = gRegistry->Add(contended_monitor_obj);
@@ -1400,7 +1392,9 @@
}
void Dbg::SetJdwpLocation(JDWP::JdwpLocation* location, mirror::ArtMethod* m, uint32_t dex_pc)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_) {
if (m == nullptr) {
memset(location, 0, sizeof(*location));
} else {
@@ -1890,7 +1884,6 @@
JDWP::JdwpError Dbg::GetThreadName(JDWP::ObjectId thread_id, std::string* name) {
ScopedObjectAccessUnchecked soa(Thread::Current());
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
Thread* thread = DecodeThread(soa, thread_id, &error);
UNUSED(thread);
@@ -1920,11 +1913,8 @@
}
ScopedAssertNoThreadSuspension ants(soa.Self(), "Debugger: GetThreadGroup");
// Okay, so it's an object, but is it actually a thread?
- {
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- Thread* thread = DecodeThread(soa, thread_id, &error);
- UNUSED(thread);
- }
+ Thread* thread = DecodeThread(soa, thread_id, &error);
+ UNUSED(thread);
if (error == JDWP::ERR_THREAD_NOT_ALIVE) {
// Zombie threads are in the null group.
expandBufAddObjectId(pReply, JDWP::ObjectId(0));
@@ -2112,7 +2102,6 @@
*pSuspendStatus = JDWP::SUSPEND_STATUS_NOT_SUSPENDED;
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
Thread* thread = DecodeThread(soa, thread_id, &error);
if (error != JDWP::ERR_NONE) {
@@ -2133,7 +2122,6 @@
JDWP::JdwpError Dbg::GetThreadDebugSuspendCount(JDWP::ObjectId thread_id, JDWP::ExpandBuf* pReply) {
ScopedObjectAccess soa(Thread::Current());
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
Thread* thread = DecodeThread(soa, thread_id, &error);
if (error != JDWP::ERR_NONE) {
@@ -2146,7 +2134,6 @@
JDWP::JdwpError Dbg::Interrupt(JDWP::ObjectId thread_id) {
ScopedObjectAccess soa(Thread::Current());
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
Thread* thread = DecodeThread(soa, thread_id, &error);
if (error != JDWP::ERR_NONE) {
@@ -2225,7 +2212,6 @@
JDWP::JdwpError Dbg::GetThreadFrameCount(JDWP::ObjectId thread_id, size_t* result) {
ScopedObjectAccess soa(Thread::Current());
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
*result = 0;
Thread* thread = DecodeThread(soa, thread_id, &error);
@@ -2251,9 +2237,7 @@
expandBufAdd4BE(buf_, frame_count_);
}
- // TODO: Enable annotalysis. We know lock is held in constructor, but abstraction confuses
- // annotalysis.
- virtual bool VisitFrame() NO_THREAD_SAFETY_ANALYSIS {
+ bool VisitFrame() OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
if (GetMethod()->IsRuntimeMethod()) {
return true; // The debugger can't do anything useful with a frame that has no Method*.
}
@@ -2280,7 +2264,6 @@
};
ScopedObjectAccessUnchecked soa(Thread::Current());
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
Thread* thread = DecodeThread(soa, thread_id, &error);
if (error != JDWP::ERR_NONE) {
@@ -2387,17 +2370,13 @@
JDWP::JdwpError Dbg::GetThisObject(JDWP::ObjectId thread_id, JDWP::FrameId frame_id,
JDWP::ObjectId* result) {
ScopedObjectAccessUnchecked soa(Thread::Current());
- Thread* thread;
- {
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- JDWP::JdwpError error;
- thread = DecodeThread(soa, thread_id, &error);
- if (error != JDWP::ERR_NONE) {
- return error;
- }
- if (!IsSuspendedForDebugger(soa, thread)) {
- return JDWP::ERR_THREAD_NOT_SUSPENDED;
- }
+ JDWP::JdwpError error;
+ Thread* thread = DecodeThread(soa, thread_id, &error);
+ if (error != JDWP::ERR_NONE) {
+ return error;
+ }
+ if (!IsSuspendedForDebugger(soa, thread)) {
+ return JDWP::ERR_THREAD_NOT_SUSPENDED;
}
std::unique_ptr<Context> context(Context::Create());
GetThisVisitor visitor(thread, context.get(), frame_id);
@@ -2444,17 +2423,13 @@
JDWP::FrameId frame_id = request->ReadFrameId();
ScopedObjectAccessUnchecked soa(Thread::Current());
- Thread* thread;
- {
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- JDWP::JdwpError error;
- thread = DecodeThread(soa, thread_id, &error);
- if (error != JDWP::ERR_NONE) {
- return error;
- }
- if (!IsSuspendedForDebugger(soa, thread)) {
- return JDWP::ERR_THREAD_NOT_SUSPENDED;
- }
+ JDWP::JdwpError error;
+ Thread* thread = DecodeThread(soa, thread_id, &error);
+ if (error != JDWP::ERR_NONE) {
+ return error;
+ }
+ if (!IsSuspendedForDebugger(soa, thread)) {
+ return JDWP::ERR_THREAD_NOT_SUSPENDED;
}
// Find the frame with the given frame_id.
std::unique_ptr<Context> context(Context::Create());
@@ -2475,7 +2450,7 @@
size_t width = Dbg::GetTagWidth(reqSigByte);
uint8_t* ptr = expandBufAddSpace(pReply, width + 1);
- JDWP::JdwpError error = Dbg::GetLocalValue(visitor, soa, slot, reqSigByte, ptr, width);
+ error = Dbg::GetLocalValue(visitor, soa, slot, reqSigByte, ptr, width);
if (error != JDWP::ERR_NONE) {
return error;
}
@@ -2619,17 +2594,13 @@
JDWP::FrameId frame_id = request->ReadFrameId();
ScopedObjectAccessUnchecked soa(Thread::Current());
- Thread* thread;
- {
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- JDWP::JdwpError error;
- thread = DecodeThread(soa, thread_id, &error);
- if (error != JDWP::ERR_NONE) {
- return error;
- }
- if (!IsSuspendedForDebugger(soa, thread)) {
- return JDWP::ERR_THREAD_NOT_SUSPENDED;
- }
+ JDWP::JdwpError error;
+ Thread* thread = DecodeThread(soa, thread_id, &error);
+ if (error != JDWP::ERR_NONE) {
+ return error;
+ }
+ if (!IsSuspendedForDebugger(soa, thread)) {
+ return JDWP::ERR_THREAD_NOT_SUSPENDED;
}
// Find the frame with the given frame_id.
std::unique_ptr<Context> context(Context::Create());
@@ -2648,7 +2619,7 @@
uint64_t value = request->ReadValue(width);
VLOG(jdwp) << " --> slot " << slot << " " << sigByte << " " << value;
- JDWP::JdwpError error = Dbg::SetLocalValue(visitor, slot, sigByte, value, width);
+ error = Dbg::SetLocalValue(visitor, slot, sigByte, value, width);
if (error != JDWP::ERR_NONE) {
return error;
}
@@ -3495,10 +3466,7 @@
self_suspend_(false),
other_suspend_(false) {
ScopedObjectAccessUnchecked soa(self);
- {
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
- thread_ = DecodeThread(soa, thread_id, &error_);
- }
+ thread_ = DecodeThread(soa, thread_id, &error_);
if (error_ == JDWP::ERR_NONE) {
if (thread_ == soa.Self()) {
self_suspend_ = true;
@@ -3668,7 +3636,6 @@
void Dbg::UnconfigureStep(JDWP::ObjectId thread_id) {
ScopedObjectAccessUnchecked soa(Thread::Current());
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
Thread* thread = DecodeThread(soa, thread_id, &error);
if (error == JDWP::ERR_NONE) {
@@ -3718,7 +3685,6 @@
Thread* self = Thread::Current();
{
ScopedObjectAccessUnchecked soa(self);
- MutexLock mu(soa.Self(), *Locks::thread_list_lock_);
JDWP::JdwpError error;
targetThread = DecodeThread(soa, thread_id, &error);
if (error != JDWP::ERR_NONE) {
diff --git a/runtime/jdwp/object_registry.cc b/runtime/jdwp/object_registry.cc
index 99a005d..a42a58f 100644
--- a/runtime/jdwp/object_registry.cc
+++ b/runtime/jdwp/object_registry.cc
@@ -50,6 +50,10 @@
Thread* const self = Thread::Current();
self->AssertNoPendingException();
+ // Object::IdentityHashCode may cause these locks to be held so check we do not already
+ // hold them.
+ Locks::thread_list_lock_->AssertNotHeld(self);
+ Locks::thread_suspend_count_lock_->AssertNotHeld(self);
StackHandleScope<1> hs(self);
Handle<mirror::Object> obj_h(hs.NewHandle(o));
diff --git a/runtime/jdwp/object_registry.h b/runtime/jdwp/object_registry.h
index 0693f33..27a4e55 100644
--- a/runtime/jdwp/object_registry.h
+++ b/runtime/jdwp/object_registry.h
@@ -62,9 +62,13 @@
ObjectRegistry();
JDWP::ObjectId Add(mirror::Object* o)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_);
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
JDWP::RefTypeId AddRefType(mirror::Class* c)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(Locks::thread_list_lock_);
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
template<typename T> T Get(JDWP::ObjectId id, JDWP::JdwpError* error)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
@@ -96,7 +100,9 @@
private:
JDWP::ObjectId InternalAdd(mirror::Object* o)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
- LOCKS_EXCLUDED(lock_, Locks::thread_list_lock_);
+ LOCKS_EXCLUDED(lock_,
+ Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
mirror::Object* InternalGet(JDWP::ObjectId id, JDWP::JdwpError* error)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index bfd5d4d..343c9bc 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -111,7 +111,10 @@
Object* Clone(Thread* self) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
- int32_t IdentityHashCode() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ int32_t IdentityHashCode() const
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
static MemberOffset MonitorOffset() {
return OFFSET_OF_OBJECT_MEMBER(Object, monitor_);