Reviewers: kozyatinskiy, Yang,
Description:
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
Please review this at https://codereview.chromium.org/1321263002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+30, -0 lines):
M src/debug/debug.cc
M test/cctest/test-debug.cc
Index: src/debug/debug.cc
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index
de7cc2315d02397e85f54388772acf6f4da68b67..ce6b785f6ef70ba416c6ea2a3b1975cf17892844
100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -1815,6 +1815,7 @@ void Debug::OnException(Handle<Object> exception,
Handle<Object> promise) {
void Debug::OnCompileError(Handle<Script> script) {
if (ignore_events()) return;
+ SuppressDebug while_processing(this);
if (in_debug_scope()) {
ProcessCompileEventInDebugScope(v8::CompileError, script);
@@ -1857,6 +1858,7 @@ void Debug::OnDebugBreak(Handle<Object>
break_points_hit,
void Debug::OnBeforeCompile(Handle<Script> script) {
if (in_debug_scope() || ignore_events()) return;
+ SuppressDebug while_processing(this);
HandleScope scope(isolate_);
DebugScope debug_scope(this);
@@ -1878,6 +1880,7 @@ void Debug::OnBeforeCompile(Handle<Script> script) {
// Handle debugger actions when a new script is compiled.
void Debug::OnAfterCompile(Handle<Script> script) {
if (ignore_events()) return;
+ SuppressDebug while_processing(this);
if (in_debug_scope()) {
ProcessCompileEventInDebugScope(v8::AfterCompile, script);
@@ -1974,6 +1977,9 @@ void Debug::CallEventCallback(v8::DebugEvent event,
Handle<Object> exec_state,
Handle<Object> event_data,
v8::Debug::ClientData* client_data) {
+ // Prevent other interrupts from triggering, for example API callbacks,
+ // while dispatching message handler callbacks.
+ PostponeInterruptsScope postpone(isolate_);
bool previous = in_debug_event_listener_;
in_debug_event_listener_ = true;
if (event_listener_->IsForeign()) {
Index: test/cctest/test-debug.cc
diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc
index
1e3f0ab707eab90451e4c762516ad0909bf53b6e..e2ba3f5d1c469f60b97e06a5df08a8b879b6055d
100644
--- a/test/cctest/test-debug.cc
+++ b/test/cctest/test-debug.cc
@@ -7626,3 +7626,27 @@ TEST(DebugBreakInLexicalScopes) {
"x * y",
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);");
+}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
