Commit 70dc24c2 authored by yurys's avatar yurys Committed by Commit bot

Postpone interrupts while dipatching debugger events to listeners

The interrupts are already postponed in message handlers [1]. This CL aligns debug event listener (the mechanism that is actually used in Chrome DevTools) implementation with that. Handling interrupts on events like v8::AfterCompile leads to crashes like the one in the lined bug. This happens because in the interrupt handler we may change debugger state.

[1] https://codereview.chromium.org/309533009/diff/40001/src/debug.cc

BUG=chromium:520702
LOG=Y

Review URL: https://codereview.chromium.org/1321263002

Cr-Commit-Position: refs/heads/master@{#30488}
parent 206f12ab
...@@ -1815,6 +1815,7 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) { ...@@ -1815,6 +1815,7 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
void Debug::OnCompileError(Handle<Script> script) { void Debug::OnCompileError(Handle<Script> script) {
if (ignore_events()) return; if (ignore_events()) return;
SuppressDebug while_processing(this);
if (in_debug_scope()) { if (in_debug_scope()) {
ProcessCompileEventInDebugScope(v8::CompileError, script); ProcessCompileEventInDebugScope(v8::CompileError, script);
...@@ -1857,6 +1858,7 @@ void Debug::OnDebugBreak(Handle<Object> break_points_hit, ...@@ -1857,6 +1858,7 @@ void Debug::OnDebugBreak(Handle<Object> break_points_hit,
void Debug::OnBeforeCompile(Handle<Script> script) { void Debug::OnBeforeCompile(Handle<Script> script) {
if (in_debug_scope() || ignore_events()) return; if (in_debug_scope() || ignore_events()) return;
SuppressDebug while_processing(this);
HandleScope scope(isolate_); HandleScope scope(isolate_);
DebugScope debug_scope(this); DebugScope debug_scope(this);
...@@ -1878,6 +1880,7 @@ void Debug::OnBeforeCompile(Handle<Script> script) { ...@@ -1878,6 +1880,7 @@ void Debug::OnBeforeCompile(Handle<Script> script) {
// Handle debugger actions when a new script is compiled. // Handle debugger actions when a new script is compiled.
void Debug::OnAfterCompile(Handle<Script> script) { void Debug::OnAfterCompile(Handle<Script> script) {
if (ignore_events()) return; if (ignore_events()) return;
SuppressDebug while_processing(this);
if (in_debug_scope()) { if (in_debug_scope()) {
ProcessCompileEventInDebugScope(v8::AfterCompile, script); ProcessCompileEventInDebugScope(v8::AfterCompile, script);
...@@ -1974,6 +1977,9 @@ void Debug::CallEventCallback(v8::DebugEvent event, ...@@ -1974,6 +1977,9 @@ void Debug::CallEventCallback(v8::DebugEvent event,
Handle<Object> exec_state, Handle<Object> exec_state,
Handle<Object> event_data, Handle<Object> event_data,
v8::Debug::ClientData* client_data) { v8::Debug::ClientData* client_data) {
// Prevent other interrupts from triggering, for example API callbacks,
// while dispatching event listners.
PostponeInterruptsScope postpone(isolate_);
bool previous = in_debug_event_listener_; bool previous = in_debug_event_listener_;
in_debug_event_listener_ = true; in_debug_event_listener_ = true;
if (event_listener_->IsForeign()) { if (event_listener_->IsForeign()) {
...@@ -2007,7 +2013,6 @@ void Debug::ProcessCompileEventInDebugScope(v8::DebugEvent event, ...@@ -2007,7 +2013,6 @@ void Debug::ProcessCompileEventInDebugScope(v8::DebugEvent event,
Handle<Script> script) { Handle<Script> script) {
if (event_listener_.is_null()) return; if (event_listener_.is_null()) return;
SuppressDebug while_processing(this);
DebugScope debug_scope(this); DebugScope debug_scope(this);
if (debug_scope.failed()) return; if (debug_scope.failed()) return;
......
...@@ -7626,3 +7626,27 @@ TEST(DebugBreakInLexicalScopes) { ...@@ -7626,3 +7626,27 @@ TEST(DebugBreakInLexicalScopes) {
"x * y", "x * y",
30); 30);
} }
static int after_compile_handler_depth = 0;
static void HandleInterrupt(v8::Isolate* isolate, void* data) {
CHECK_EQ(0, after_compile_handler_depth);
}
static void NoInterruptsOnDebugEvent(
const v8::Debug::EventDetails& event_details) {
if (event_details.GetEvent() != v8::AfterCompile) return;
++after_compile_handler_depth;
// Do not allow nested AfterCompile events.
CHECK(after_compile_handler_depth <= 1);
v8::Isolate* isolate = event_details.GetEventContext()->GetIsolate();
isolate->RequestInterrupt(&HandleInterrupt, nullptr);
CompileRun("function foo() {}; foo();");
--after_compile_handler_depth;
}
TEST(NoInterruptsInDebugListener) {
DebugLocalContext env;
v8::Debug::SetDebugEventListener(NoInterruptsOnDebugEvent);
CompileRun("void(0);");
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment