JDWP: more GC safety
Ensures GC safety when keeping references that may be moved by GC:
- SingleStepControl: stores ArtMethod* in a GcRoot
- ModBasket: stores references in a StackHandleScope
Bug: 18166750
Change-Id: If2b6f9ecff4cf469b50487fd863319fdfa9b9f37
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 811d15a..80fd18e 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -411,7 +411,7 @@
}
void SingleStepControl::VisitRoots(RootVisitor* visitor, const RootInfo& root_info) {
- visitor->VisitRootIfNonNull(reinterpret_cast<mirror::Object**>(&method_), root_info);
+ method_.VisitRootIfNonNull(visitor, root_info);
}
void SingleStepControl::AddDexPc(uint32_t dex_pc) {
@@ -2928,10 +2928,11 @@
if (!IsDebuggerActive()) {
return;
}
- StackHandleScope<1> handle_scope(Thread::Current());
+ Thread* const self = Thread::Current();
+ StackHandleScope<1> handle_scope(self);
Handle<mirror::Throwable> h_exception(handle_scope.NewHandle(exception_object));
std::unique_ptr<Context> context(Context::Create());
- CatchLocationFinder clf(Thread::Current(), h_exception, context.get());
+ CatchLocationFinder clf(self, h_exception, context.get());
clf.WalkStack(/* include_transitions */ false);
JDWP::EventLocation exception_throw_location;
SetEventLocation(&exception_throw_location, clf.GetThrowMethod(), clf.GetThrowDexPc());
@@ -3980,7 +3981,7 @@
Handle<mirror::Object> object_result = hs.NewHandle(is_object_result ? result.GetL() : nullptr);
Handle<mirror::Throwable> exception = hs.NewHandle(soa.Self()->GetException());
soa.Self()->ClearException();
- pReq->exception = gRegistry->Add(exception.Get());
+ pReq->exception = gRegistry->Add(exception);
if (pReq->exception != 0) {
VLOG(jdwp) << " JDWP invocation returning with exception=" << exception.Get()
<< " " << exception->Dump();
diff --git a/runtime/debugger.h b/runtime/debugger.h
index 789a0a4..811d345 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -109,8 +109,8 @@
return stack_depth_;
}
- mirror::ArtMethod* GetMethod() const {
- return method_;
+ mirror::ArtMethod* GetMethod() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ return method_.Read();
}
const std::set<uint32_t>& GetDexPcs() const {
@@ -138,7 +138,7 @@
// set of DEX pcs associated to the source line number where the suspension occurred.
// This is used to support SD_INTO and SD_OVER single-step depths so we detect when a single-step
// causes the execution of an instruction in a different method or at a different line number.
- mirror::ArtMethod* method_;
+ GcRoot<mirror::ArtMethod> method_;
std::set<uint32_t> dex_pcs_;
DISALLOW_COPY_AND_ASSIGN(SingleStepControl);
diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc
index ab3f2e4..ff75268 100644
--- a/runtime/jdwp/jdwp_event.cc
+++ b/runtime/jdwp/jdwp_event.cc
@@ -32,6 +32,8 @@
#include "scoped_thread_state_change.h"
#include "thread-inl.h"
+#include "handle_scope-inl.h"
+
/*
General notes:
@@ -108,20 +110,32 @@
* Stuff to compare against when deciding if a mod matches. Only the
* values for mods valid for the event being evaluated will be filled in.
* The rest will be zeroed.
+ * Must be allocated on the stack only. This is enforced by removing the
+ * operator new.
*/
struct ModBasket {
- ModBasket() : pLoc(nullptr), thread(nullptr), locationClass(nullptr), exceptionClass(nullptr),
- caught(false), field(nullptr), thisPtr(nullptr) { }
+ explicit ModBasket(Thread* self)
+ : hs(self), pLoc(nullptr), thread(self),
+ locationClass(hs.NewHandle<mirror::Class>(nullptr)),
+ exceptionClass(hs.NewHandle<mirror::Class>(nullptr)),
+ caught(false),
+ field(nullptr),
+ thisPtr(hs.NewHandle<mirror::Object>(nullptr)) { }
- const EventLocation* pLoc; /* LocationOnly */
- std::string className; /* ClassMatch/ClassExclude */
- Thread* thread; /* ThreadOnly */
- mirror::Class* locationClass; /* ClassOnly */
- mirror::Class* exceptionClass; /* ExceptionOnly */
- bool caught; /* ExceptionOnly */
- ArtField* field; /* FieldOnly */
- mirror::Object* thisPtr; /* InstanceOnly */
+ StackHandleScope<3> hs;
+ const EventLocation* pLoc; /* LocationOnly */
+ std::string className; /* ClassMatch/ClassExclude */
+ Thread* const thread; /* ThreadOnly */
+ MutableHandle<mirror::Class> locationClass; /* ClassOnly */
+ MutableHandle<mirror::Class> exceptionClass; /* ExceptionOnly */
+ bool caught; /* ExceptionOnly */
+ ArtField* field; /* FieldOnly */
+ MutableHandle<mirror::Object> thisPtr; /* InstanceOnly */
/* nothing for StepOnly -- handled differently */
+
+ private:
+ DISALLOW_ALLOCATION(); // forbids allocation on the heap.
+ DISALLOW_IMPLICIT_CONSTRUCTORS(ModBasket);
};
static bool NeedsFullDeoptimization(JdwpEventKind eventKind) {
@@ -457,7 +471,7 @@
}
break;
case MK_CLASS_ONLY:
- if (!Dbg::MatchType(basket.locationClass, pMod->classOnly.refTypeId)) {
+ if (!Dbg::MatchType(basket.locationClass.Get(), pMod->classOnly.refTypeId)) {
return false;
}
break;
@@ -478,7 +492,7 @@
break;
case MK_EXCEPTION_ONLY:
if (pMod->exceptionOnly.refTypeId != 0 &&
- !Dbg::MatchType(basket.exceptionClass, pMod->exceptionOnly.refTypeId)) {
+ !Dbg::MatchType(basket.exceptionClass.Get(), pMod->exceptionOnly.refTypeId)) {
return false;
}
if ((basket.caught && !pMod->exceptionOnly.caught) ||
@@ -497,7 +511,7 @@
}
break;
case MK_INSTANCE_ONLY:
- if (!Dbg::MatchInstance(pMod->instanceOnly.objectId, basket.thisPtr)) {
+ if (!Dbg::MatchInstance(pMod->instanceOnly.objectId, basket.thisPtr.Get())) {
return false;
}
break;
@@ -825,12 +839,11 @@
DCHECK(pLoc->method != nullptr);
DCHECK_EQ(pLoc->method->IsStatic(), thisPtr == nullptr);
- ModBasket basket;
+ ModBasket basket(Thread::Current());
basket.pLoc = pLoc;
- basket.locationClass = pLoc->method->GetDeclaringClass();
- basket.thisPtr = thisPtr;
- basket.thread = Thread::Current();
- basket.className = Dbg::GetClassName(basket.locationClass);
+ basket.locationClass.Assign(pLoc->method->GetDeclaringClass());
+ basket.thisPtr.Assign(thisPtr);
+ basket.className = Dbg::GetClassName(basket.locationClass.Get());
/*
* On rare occasions we may need to execute interpreted code in the VM
@@ -924,16 +937,15 @@
DCHECK_EQ(fieldValue != nullptr, is_modification);
DCHECK_EQ(field->IsStatic(), this_object == nullptr);
- ModBasket basket;
+ ModBasket basket(Thread::Current());
basket.pLoc = pLoc;
- basket.locationClass = pLoc->method->GetDeclaringClass();
- basket.thisPtr = this_object;
- basket.thread = Thread::Current();
- basket.className = Dbg::GetClassName(basket.locationClass);
+ basket.locationClass.Assign(pLoc->method->GetDeclaringClass());
+ basket.thisPtr.Assign(this_object);
+ basket.className = Dbg::GetClassName(basket.locationClass.Get());
basket.field = field;
if (InvokeInProgress()) {
- VLOG(jdwp) << "Not posting field event during invoke";
+ VLOG(jdwp) << "Not posting field event during invoke (" << basket.className << ")";
return;
}
@@ -975,7 +987,7 @@
uint8_t tag;
{
ScopedObjectAccessUnchecked soa(Thread::Current());
- tag = Dbg::TagFromObject(soa, basket.thisPtr);
+ tag = Dbg::TagFromObject(soa, basket.thisPtr.Get());
}
for (const JdwpEvent* pEvent : match_list) {
@@ -1028,8 +1040,7 @@
return;
}
- ModBasket basket;
- basket.thread = thread;
+ ModBasket basket(thread);
std::vector<JdwpEvent*> match_list;
const JdwpEventKind match_kind = (start) ? EK_THREAD_START : EK_THREAD_DEATH;
@@ -1106,18 +1117,15 @@
VLOG(jdwp) << "Unexpected: exception event with empty throw location";
}
- ModBasket basket;
+ ModBasket basket(Thread::Current());
basket.pLoc = pThrowLoc;
if (pThrowLoc->method != nullptr) {
- basket.locationClass = pThrowLoc->method->GetDeclaringClass();
- } else {
- basket.locationClass = nullptr;
+ basket.locationClass.Assign(pThrowLoc->method->GetDeclaringClass());
}
- basket.thread = Thread::Current();
- basket.className = Dbg::GetClassName(basket.locationClass);
- basket.exceptionClass = exception_object->GetClass();
+ basket.className = Dbg::GetClassName(basket.locationClass.Get());
+ basket.exceptionClass.Assign(exception_object->GetClass());
basket.caught = (pCatchLoc->method != 0);
- basket.thisPtr = thisPtr;
+ basket.thisPtr.Assign(thisPtr);
/* don't try to post an exception caused by the debugger */
if (InvokeInProgress()) {
@@ -1188,10 +1196,9 @@
void JdwpState::PostClassPrepare(mirror::Class* klass) {
DCHECK(klass != nullptr);
- ModBasket basket;
- basket.locationClass = klass;
- basket.thread = Thread::Current();
- basket.className = Dbg::GetClassName(basket.locationClass);
+ ModBasket basket(Thread::Current());
+ basket.locationClass.Assign(klass);
+ basket.className = Dbg::GetClassName(basket.locationClass.Get());
/* suppress class prep caused by debugger */
if (InvokeInProgress()) {
@@ -1214,7 +1221,7 @@
// debuggers seem to like that. There might be some advantage to honesty,
// since the class may not yet be verified.
int status = JDWP::CS_VERIFIED | JDWP::CS_PREPARED;
- JDWP::JdwpTypeTag tag = Dbg::GetTypeTag(basket.locationClass);
+ JDWP::JdwpTypeTag tag = Dbg::GetTypeTag(basket.locationClass.Get());
std::string temp;
std::string signature(basket.locationClass->GetDescriptor(&temp));
diff --git a/runtime/jdwp/object_registry.cc b/runtime/jdwp/object_registry.cc
index a42a58f..2b28f7d 100644
--- a/runtime/jdwp/object_registry.cc
+++ b/runtime/jdwp/object_registry.cc
@@ -36,17 +36,45 @@
}
JDWP::RefTypeId ObjectRegistry::AddRefType(mirror::Class* c) {
- return InternalAdd(c);
+ return Add(c);
+}
+
+JDWP::RefTypeId ObjectRegistry::AddRefType(Handle<mirror::Class> c_h) {
+ return Add(c_h);
}
JDWP::ObjectId ObjectRegistry::Add(mirror::Object* o) {
- return InternalAdd(o);
-}
-
-JDWP::ObjectId ObjectRegistry::InternalAdd(mirror::Object* o) {
if (o == nullptr) {
return 0;
}
+ Thread* const self = Thread::Current();
+ StackHandleScope<1> hs(self);
+ return InternalAdd(hs.NewHandle(o));
+}
+
+// Template instantiations must be declared below.
+template<class T>
+JDWP::ObjectId ObjectRegistry::Add(Handle<T> obj_h) {
+ if (obj_h.Get() == nullptr) {
+ return 0;
+ }
+ return InternalAdd(obj_h);
+}
+
+// Explicit template instantiation.
+template
+SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::thread_suspend_count_lock_)
+JDWP::ObjectId ObjectRegistry::Add(Handle<mirror::Object> obj_h);
+
+template
+SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::thread_suspend_count_lock_)
+JDWP::ObjectId ObjectRegistry::Add(Handle<mirror::Throwable> obj_h);
+
+template<class T>
+JDWP::ObjectId ObjectRegistry::InternalAdd(Handle<T> obj_h) {
+ CHECK(obj_h.Get() != nullptr);
Thread* const self = Thread::Current();
self->AssertNoPendingException();
@@ -55,9 +83,6 @@
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));
-
// Call IdentityHashCode here to avoid a lock level violation between lock_ and monitor_lock.
int32_t identity_hash_code = obj_h->IdentityHashCode();
diff --git a/runtime/jdwp/object_registry.h b/runtime/jdwp/object_registry.h
index 27a4e55..4c149cd 100644
--- a/runtime/jdwp/object_registry.h
+++ b/runtime/jdwp/object_registry.h
@@ -23,6 +23,7 @@
#include <map>
#include "base/casts.h"
+#include "handle.h"
#include "jdwp/jdwp.h"
#include "safe_map.h"
@@ -65,11 +66,23 @@
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_,
Locks::thread_suspend_count_lock_);
+ template<class T>
+ JDWP::ObjectId Add(Handle<T> obj_h)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ LOCKS_EXCLUDED(Locks::thread_list_lock_,
+ Locks::thread_suspend_count_lock_);
+
+ JDWP::RefTypeId AddRefType(Handle<mirror::Class> c_h)
+ 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_) {
if (id == 0) {
@@ -98,7 +111,8 @@
jobject GetJObject(JDWP::ObjectId id) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
private:
- JDWP::ObjectId InternalAdd(mirror::Object* o)
+ template<class T>
+ JDWP::ObjectId InternalAdd(Handle<T> obj_h)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
LOCKS_EXCLUDED(lock_,
Locks::thread_list_lock_,
