Don't allow thread suspension in ProfilingInfo::Create
AddSamples calls ProfilingInfo::Create which would occasionally
transition to suspended and wait for code cache GC to complete.
This CL removes the thread state change and
WaitForPotentialCollectionToComplete which caused thread suspension.
The thread suspension caused occasional stale object references in
InvokeVirtualOrInterface since moving GC could occur.
If this_object became stale, we would put it's stale class into the
profile info, resulting in a GC crash.
Bug: 26086970
Change-Id: I5a86561098d72b7be80e8a3bcf3d8401403a0b00
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 9f61449..7d60264 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -931,6 +931,9 @@
ArtMethod* caller,
uint32_t dex_pc,
ArtMethod* callee) const {
+ // We can not have thread suspension since that would cause the this_object parameter to
+ // potentially become a dangling pointer. An alternative could be to put it in a handle instead.
+ ScopedAssertNoThreadSuspension ants(thread, __FUNCTION__);
for (InstrumentationListener* listener : invoke_virtual_or_interface_listeners_) {
if (listener != nullptr) {
listener->InvokeVirtualOrInterface(thread, this_object, caller, dex_pc, callee);
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 5e0a11d..b29245f 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -104,6 +104,7 @@
ArtMethod* caller,
uint32_t dex_pc,
ArtMethod* callee)
+ REQUIRES(Roles::uninterruptible_)
SHARED_REQUIRES(Locks::mutator_lock_) = 0;
};
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 5db699b..2d575bd 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -674,9 +674,7 @@
size_t profile_info_size = RoundUp(
sizeof(ProfilingInfo) + sizeof(InlineCache) * entries.size(),
sizeof(void*));
- ScopedThreadSuspension sts(self, kSuspended);
MutexLock mu(self, lock_);
- WaitForPotentialCollectionToComplete(self);
// Check whether some other thread has concurrently created it.
ProfilingInfo* info = method->GetProfilingInfo(sizeof(void*));
diff --git a/runtime/jit/jit_instrumentation.cc b/runtime/jit/jit_instrumentation.cc
index 6531325..4cbaf2c 100644
--- a/runtime/jit/jit_instrumentation.cc
+++ b/runtime/jit/jit_instrumentation.cc
@@ -177,9 +177,8 @@
ArtMethod* caller,
uint32_t dex_pc,
ArtMethod* callee ATTRIBUTE_UNUSED) {
- instrumentation_cache_->AddSamples(thread, caller, 1);
// We make sure we cannot be suspended, as the profiling info can be concurrently deleted.
- thread->StartAssertNoThreadSuspension("Instrumenting invoke");
+ instrumentation_cache_->AddSamples(thread, caller, 1);
DCHECK(this_object != nullptr);
ProfilingInfo* info = caller->GetProfilingInfo(sizeof(void*));
if (info != nullptr) {
@@ -188,7 +187,6 @@
Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(caller->GetDeclaringClass());
info->AddInvokeInfo(dex_pc, this_object->GetClass());
}
- thread->EndAssertNoThreadSuspension(nullptr);
}
void JitInstrumentationCache::WaitForCompilationToFinish(Thread* self) {
diff --git a/runtime/jit/jit_instrumentation.h b/runtime/jit/jit_instrumentation.h
index 1f96d59..15969e4 100644
--- a/runtime/jit/jit_instrumentation.h
+++ b/runtime/jit/jit_instrumentation.h
@@ -78,7 +78,9 @@
ArtMethod* caller,
uint32_t dex_pc,
ArtMethod* callee)
- OVERRIDE SHARED_REQUIRES(Locks::mutator_lock_);
+ OVERRIDE
+ REQUIRES(Roles::uninterruptible_)
+ SHARED_REQUIRES(Locks::mutator_lock_);
static constexpr uint32_t kJitEvents =
instrumentation::Instrumentation::kMethodEntered |