Fix some HandleScope bugs and add corresponding checks
Some places were creating or destroying handle scopes without holding
the mutator lock. This can cause GC crashes if thread roots are being
marked or hprof dumps to also fail.
Also added checks to catch some of these errors.
Bug: 23468617
Change-Id: I1a2d615923484cfc25014967656775c445aa3f1f
diff --git a/compiler/driver/compiler_driver_test.cc b/compiler/driver/compiler_driver_test.cc
index e35d07d..a5ace0b 100644
--- a/compiler/driver/compiler_driver_test.cc
+++ b/compiler/driver/compiler_driver_test.cc
@@ -210,8 +210,8 @@
CompileAll(class_loader);
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- StackHandleScope<1> hs(self);
ScopedObjectAccess soa(self);
+ StackHandleScope<1> hs(self);
Handle<mirror::ClassLoader> h_loader(hs.NewHandle(
reinterpret_cast<mirror::ClassLoader*>(self->DecodeJObject(class_loader))));
mirror::Class* klass = class_linker->FindClass(self, "LStaticLeafMethods;", h_loader);
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index 2a76991..1db3063 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -540,11 +540,14 @@
CompilerDriver* compiler_driver,
const DexCompilationUnit& dex_compilation_unit,
PassObserver* pass_observer) const {
- StackHandleScopeCollection handles(Thread::Current());
+ ScopedObjectAccess soa(Thread::Current());
+ StackHandleScopeCollection handles(soa.Self());
+ soa.Self()->TransitionFromRunnableToSuspended(kNative);
RunOptimizations(graph, compiler_driver, compilation_stats_.get(),
dex_compilation_unit, pass_observer, &handles);
if (graph->HasTryCatch()) {
+ soa.Self()->TransitionFromSuspendedToRunnable();
return nullptr;
}
@@ -582,6 +585,8 @@
ArrayRef<const uint8_t>(*codegen->GetAssembler()->cfi().data()),
ArrayRef<const LinkerPatch>(linker_patches));
pass_observer->DumpDisassembly();
+
+ soa.Self()->TransitionFromSuspendedToRunnable();
return compiled_method;
}
diff --git a/runtime/gc/reference_queue_test.cc b/runtime/gc/reference_queue_test.cc
index 888c0d2..ab921d9 100644
--- a/runtime/gc/reference_queue_test.cc
+++ b/runtime/gc/reference_queue_test.cc
@@ -27,11 +27,11 @@
TEST_F(ReferenceQueueTest, EnqueueDequeue) {
Thread* self = Thread::Current();
+ ScopedObjectAccess soa(self);
StackHandleScope<20> hs(self);
Mutex lock("Reference queue lock");
ReferenceQueue queue(&lock);
ASSERT_TRUE(queue.IsEmpty());
- ScopedObjectAccess soa(self);
ASSERT_EQ(queue.GetLength(), 0U);
auto ref_class = hs.NewHandle(
Runtime::Current()->GetClassLinker()->FindClass(self, "Ljava/lang/ref/WeakReference;",
@@ -58,10 +58,10 @@
TEST_F(ReferenceQueueTest, Dump) {
Thread* self = Thread::Current();
+ ScopedObjectAccess soa(self);
StackHandleScope<20> hs(self);
Mutex lock("Reference queue lock");
ReferenceQueue queue(&lock);
- ScopedObjectAccess soa(self);
queue.Dump(LOG(INFO));
auto weak_ref_class = hs.NewHandle(
Runtime::Current()->GetClassLinker()->FindClass(self, "Ljava/lang/ref/WeakReference;",
diff --git a/runtime/handle_scope-inl.h b/runtime/handle_scope-inl.h
index 222083b..ca206ef 100644
--- a/runtime/handle_scope-inl.h
+++ b/runtime/handle_scope-inl.h
@@ -19,8 +19,9 @@
#include "handle_scope.h"
+#include "base/mutex.h"
#include "handle.h"
-#include "thread.h"
+#include "thread-inl.h"
#include "verify_object-inl.h"
namespace art {
@@ -29,6 +30,9 @@
inline StackHandleScope<kNumReferences>::StackHandleScope(Thread* self, mirror::Object* fill_value)
: HandleScope(self->GetTopHandleScope(), kNumReferences), self_(self), pos_(0) {
DCHECK_EQ(self, Thread::Current());
+ if (kDebugLocking) {
+ Locks::mutator_lock_->AssertSharedHeld(Thread::Current());
+ }
static_assert(kNumReferences >= 1, "StackHandleScope must contain at least 1 reference");
// TODO: Figure out how to use a compile assert.
CHECK_EQ(&storage_[0], GetReferences());
@@ -42,6 +46,9 @@
inline StackHandleScope<kNumReferences>::~StackHandleScope() {
HandleScope* top_handle_scope = self_->PopHandleScope();
DCHECK_EQ(top_handle_scope, this);
+ if (kDebugLocking) {
+ Locks::mutator_lock_->AssertSharedHeld(self_);
+ }
}
inline size_t HandleScope::SizeOf(uint32_t num_references) {
@@ -59,6 +66,9 @@
inline mirror::Object* HandleScope::GetReference(size_t i) const {
DCHECK_LT(i, number_of_references_);
+ if (kDebugLocking) {
+ Locks::mutator_lock_->AssertSharedHeld(Thread::Current());
+ }
return GetReferences()[i].AsMirrorPtr();
}
@@ -73,6 +83,9 @@
}
inline void HandleScope::SetReference(size_t i, mirror::Object* object) {
+ if (kDebugLocking) {
+ Locks::mutator_lock_->AssertSharedHeld(Thread::Current());
+ }
DCHECK_LT(i, number_of_references_);
GetReferences()[i].Assign(object);
}
@@ -104,6 +117,9 @@
template<size_t kNumReferences>
inline void StackHandleScope<kNumReferences>::SetReference(size_t i, mirror::Object* object) {
+ if (kDebugLocking) {
+ Locks::mutator_lock_->AssertSharedHeld(Thread::Current());
+ }
DCHECK_LT(i, kNumReferences);
VerifyObject(object);
GetReferences()[i].Assign(object);
diff --git a/runtime/monitor_test.cc b/runtime/monitor_test.cc
index e1173bb..69112b1 100644
--- a/runtime/monitor_test.cc
+++ b/runtime/monitor_test.cc
@@ -290,15 +290,13 @@
static void CommonWaitSetup(MonitorTest* test, ClassLinker* class_linker, uint64_t create_sleep,
int64_t c_millis, bool c_expected, bool interrupt, uint64_t use_sleep,
int64_t u_millis, bool u_expected, const char* pool_name) {
+ Thread* const self = Thread::Current();
+ ScopedObjectAccess soa(self);
// First create the object we lock. String is easiest.
- StackHandleScope<3> hs(Thread::Current());
- {
- ScopedObjectAccess soa(Thread::Current());
- test->object_ = hs.NewHandle(mirror::String::AllocFromModifiedUtf8(Thread::Current(),
- "hello, world!"));
- test->watchdog_object_ = hs.NewHandle(mirror::String::AllocFromModifiedUtf8(Thread::Current(),
- "hello, world!"));
- }
+ StackHandleScope<3> hs(soa.Self());
+ test->object_ = hs.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "hello, world!"));
+ test->watchdog_object_ = hs.NewHandle(mirror::String::AllocFromModifiedUtf8(self,
+ "hello, world!"));
// Create the barrier used to synchronize.
test->barrier_ = std::unique_ptr<Barrier>(new Barrier(2));
@@ -308,23 +306,17 @@
// Fill the heap.
std::unique_ptr<StackHandleScope<kMaxHandles>> hsp;
std::vector<MutableHandle<mirror::Object>> handles;
- {
- Thread* self = Thread::Current();
- ScopedObjectAccess soa(self);
- // Our job: Fill the heap, then try Wait.
- FillHeap(self, class_linker, &hsp, &handles);
+ // Our job: Fill the heap, then try Wait.
+ FillHeap(soa.Self(), class_linker, &hsp, &handles);
- // Now release everything.
- auto it = handles.begin();
- auto end = handles.end();
+ // Now release everything.
+ for (MutableHandle<mirror::Object>& h : handles) {
+ h.Assign(nullptr);
+ }
- for ( ; it != end; ++it) {
- it->Assign(nullptr);
- }
- } // Need to drop the mutator lock to allow barriers.
-
- Thread* self = Thread::Current();
+ // Need to drop the mutator lock to allow barriers.
+ soa.Self()->TransitionFromRunnableToSuspended(kNative);
ThreadPool thread_pool(pool_name, 3);
thread_pool.AddTask(self, new CreateTask(test, create_sleep, c_millis, c_expected));
if (interrupt) {
@@ -336,19 +328,19 @@
thread_pool.StartWorkers(self);
// Wait on completion barrier.
- test->complete_barrier_->Wait(Thread::Current());
+ test->complete_barrier_->Wait(self);
test->completed_ = true;
// Wake the watchdog.
{
- ScopedObjectAccess soa(Thread::Current());
-
+ ScopedObjectAccess soa2(self);
test->watchdog_object_.Get()->MonitorEnter(self); // Lock the object.
test->watchdog_object_.Get()->NotifyAll(self); // Wake up waiting parties.
test->watchdog_object_.Get()->MonitorExit(self); // Release the lock.
}
thread_pool.StopWorkers(self);
+ soa.Self()->TransitionFromSuspendedToRunnable();
}