Fix deoptimization deadlock
Fixes a deadlock occuring during undeoptimization on debugging session end.
Before disconnecting debugger, we must unregister all requested events. We take
the event list lock to browse all these events. Some of them (as METHOD_EXIT)
may have activated deoptimization so we need to undeoptimize. During this
process, we restore all original entrypoints of every method in the stack and
notify method exit events to the instrumentation listener (see method
InstrumentationLister::MethodExited). In our case, the instrumentation listener
is the debugger. It takes the event list lock (to browse the event list and see
if this event must be posted) but hangs waiting for it. Since it's already
holding the event list lock (which is not recursive), it ends up in a deadlock
situation.
This CL fixes the way we prevent from posting method enter/exit events during
the process of deoptimization/undeoptimization. We now explicitly set a flag
indicating if deoptimization is enabled (by the debugger).
Also removes unused field in InstallStackVisitor class and remove debugger as
listener before disabling deoptimization to ensure it does not receive any
event when disconnecting.
Change-Id: I49a2ae43e86cf29094f4b462bfa754d7740d3e97
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 3c238d6..89f841e 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -594,12 +594,12 @@
MutexLock mu(Thread::Current(), *Locks::deoptimization_lock_);
gDeoptimizationRequests.clear();
}
- runtime->GetInstrumentation()->DisableDeoptimization();
runtime->GetInstrumentation()->RemoveListener(&gDebugInstrumentationListener,
instrumentation::Instrumentation::kMethodEntered |
instrumentation::Instrumentation::kMethodExited |
instrumentation::Instrumentation::kDexPcMoved |
instrumentation::Instrumentation::kExceptionCaught);
+ runtime->GetInstrumentation()->DisableDeoptimization();
gDebuggerActive = false;
gRegistry->Clear();
gDebuggerConnected = false;
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 59ffdc1..9d05169 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -145,12 +145,10 @@
static void InstrumentationInstallStack(Thread* thread, void* arg)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
struct InstallStackVisitor : public StackVisitor {
- InstallStackVisitor(Thread* thread, Context* context, uintptr_t instrumentation_exit_pc,
- bool is_deoptimization_enabled)
+ InstallStackVisitor(Thread* thread, Context* context, uintptr_t instrumentation_exit_pc)
: StackVisitor(thread, context), instrumentation_stack_(thread->GetInstrumentationStack()),
existing_instrumentation_frames_count_(instrumentation_stack_->size()),
instrumentation_exit_pc_(instrumentation_exit_pc),
- is_deoptimization_enabled_(is_deoptimization_enabled),
reached_existing_instrumentation_frames_(false), instrumentation_stack_depth_(0),
last_return_pc_(0) {
}
@@ -218,7 +216,6 @@
const size_t existing_instrumentation_frames_count_;
std::vector<uint32_t> dex_pcs_;
const uintptr_t instrumentation_exit_pc_;
- const bool is_deoptimization_enabled_;
bool reached_existing_instrumentation_frames_;
size_t instrumentation_stack_depth_;
uintptr_t last_return_pc_;
@@ -232,12 +229,11 @@
Instrumentation* instrumentation = reinterpret_cast<Instrumentation*>(arg);
UniquePtr<Context> context(Context::Create());
uintptr_t instrumentation_exit_pc = GetQuickInstrumentationExitPc();
- InstallStackVisitor visitor(thread, context.get(), instrumentation_exit_pc,
- instrumentation->IsDeoptimizationEnabled());
+ InstallStackVisitor visitor(thread, context.get(), instrumentation_exit_pc);
visitor.WalkStack(true);
CHECK_EQ(visitor.dex_pcs_.size(), thread->GetInstrumentationStack()->size());
- if (!instrumentation->IsDeoptimizationEnabled()) {
+ if (!instrumentation->ShouldNotifyMethodEnterExitEvents()) {
// Create method enter events for all methods currently on the thread's stack. We only do this
// if no debugger is attached to prevent from posting events twice.
typedef std::deque<InstrumentationStackFrame>::const_reverse_iterator It;
@@ -295,7 +291,7 @@
CHECK(m == instrumentation_frame.method_) << PrettyMethod(m);
}
SetReturnPc(instrumentation_frame.return_pc_);
- if (!instrumentation_->IsDeoptimizationEnabled()) {
+ if (!instrumentation_->ShouldNotifyMethodEnterExitEvents()) {
// Create the method exit events. As the methods didn't really exit the result is 0.
// We only do this if no debugger is attached to prevent from posting events twice.
instrumentation_->MethodExitEvent(thread_, instrumentation_frame.this_object_, m,
@@ -586,9 +582,12 @@
void Instrumentation::EnableDeoptimization() {
CHECK(deoptimized_methods_.empty());
+ CHECK_EQ(deoptimization_enabled_, false);
+ deoptimization_enabled_ = true;
}
void Instrumentation::DisableDeoptimization() {
+ CHECK_EQ(deoptimization_enabled_, true);
// If we deoptimized everything, undo it.
if (interpreter_stubs_installed_) {
UndeoptimizeEverything();
@@ -599,10 +598,12 @@
Undeoptimize(*it_begin);
}
CHECK(deoptimized_methods_.empty());
+ deoptimization_enabled_ = false;
}
-bool Instrumentation::IsDeoptimizationEnabled() const {
- return interpreter_stubs_installed_ || !deoptimized_methods_.empty();
+// Indicates if instrumentation should notify method enter/exit events to the listeners.
+bool Instrumentation::ShouldNotifyMethodEnterExitEvents() const {
+ return deoptimization_enabled_ || interpreter_stubs_installed_;
}
void Instrumentation::DeoptimizeEverything() {
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index f01add1..1ce72bd 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -105,6 +105,7 @@
have_method_entry_listeners_(false), have_method_exit_listeners_(false),
have_method_unwind_listeners_(false), have_dex_pc_listeners_(false),
have_exception_caught_listeners_(false),
+ deoptimization_enabled_(false),
interpreter_handler_table_(kMainHandlerTable),
quick_alloc_entry_points_instrumentation_counter_(0) {}
@@ -124,7 +125,7 @@
// Deoptimization.
void EnableDeoptimization() EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_);
void DisableDeoptimization() EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_);
- bool IsDeoptimizationEnabled() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ bool ShouldNotifyMethodEnterExitEvents() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
// Executes everything with interpreter.
void DeoptimizeEverything()
@@ -345,6 +346,7 @@
// only.
// TODO we need to visit these methods as roots.
std::set<mirror::ArtMethod*> deoptimized_methods_;
+ bool deoptimization_enabled_;
// Current interpreter handler table. This is updated each time the thread state flags are
// modified.