Author: [email protected]
Date: Mon May 25 00:51:04 2009
New Revision: 2035
Modified:
branches/bleeding_edge/src/debug.cc
branches/bleeding_edge/src/debug.h
branches/bleeding_edge/test/cctest/test-debug.cc
Log:
When message handler is set to NULL and there is no debugger listener the
debugger is unloaded immediately unless it's entered, in which case it will
be unloaded when last instance of EnterDebugger is destroyed.
Without the change the debugger may crash as
Debugger::EventActive(v8::Break) called from OnDebugBreak may clear current
debugger context.
Also when compilation cache was enabled debugger could fail on second
attach for the same reason(see
AfterCompileMessageWhenMessageHandlerIsReset).
BUG=12404
Review URL: http://codereview.chromium.org/115709
Modified: branches/bleeding_edge/src/debug.cc
==============================================================================
--- branches/bleeding_edge/src/debug.cc (original)
+++ branches/bleeding_edge/src/debug.cc Mon May 25 00:51:04 2009
@@ -1589,7 +1589,7 @@
bool Debugger::is_loading_debugger_ = false;
bool Debugger::never_unload_debugger_ = false;
v8::Debug::MessageHandler2 Debugger::message_handler_ = NULL;
-bool Debugger::message_handler_cleared_ = false;
+bool Debugger::debugger_unload_pending_ = false;
v8::Debug::HostDispatchHandler Debugger::host_dispatch_handler_ = NULL;
int Debugger::host_dispatch_micros_ = 100 * 1000;
DebuggerAgent* Debugger::agent_ = NULL;
@@ -1981,8 +1981,8 @@
Debug::Unload();
}
- // Clear the flag indicating that the message handler was recently
cleared.
- message_handler_cleared_ = false;
+ // Clear the flag indicating that the debugger should be unloaded.
+ debugger_unload_pending_ = false;
}
@@ -2169,17 +2169,7 @@
event_listener_data_ =
Handle<Object>::cast(GlobalHandles::Create(*data));
}
- // Unload the debugger if event listener cleared.
- if (callback->IsUndefined()) {
- UnloadDebugger();
- }
-
- // Disable the compilation cache when the debugger is active.
- if (IsDebuggerActive()) {
- CompilationCache::Disable();
- } else {
- CompilationCache::Enable();
- }
+ ListenersChanged();
}
@@ -2187,22 +2177,32 @@
ScopedLock with(debugger_access_);
message_handler_ = handler;
+ ListenersChanged();
if (handler == NULL) {
- // Indicate that the message handler was recently cleared.
- message_handler_cleared_ = true;
-
// Send an empty command to the debugger if in a break to make
JavaScript
// run again if the debugger is closed.
if (Debug::InDebugger()) {
ProcessCommand(Vector<const uint16_t>::empty());
}
}
+}
- // Disable the compilation cache when the debugger is active.
+
+void Debugger::ListenersChanged() {
if (IsDebuggerActive()) {
+ // Disable the compilation cache when the debugger is active.
CompilationCache::Disable();
} else {
CompilationCache::Enable();
+
+ // Unload the debugger if event listener and message handler cleared.
+ if (Debug::InDebugger()) {
+ // If we are in debugger set the flag to unload the debugger when
last
+ // EnterDebugger on the current stack is destroyed.
+ debugger_unload_pending_ = true;
+ } else {
+ UnloadDebugger();
+ }
}
}
Modified: branches/bleeding_edge/src/debug.h
==============================================================================
--- branches/bleeding_edge/src/debug.h (original)
+++ branches/bleeding_edge/src/debug.h Mon May 25 00:51:04 2009
@@ -629,7 +629,7 @@
ScopedLock with(debugger_access_);
// Check whether the message handler was been cleared.
- if (message_handler_cleared_) {
+ if (debugger_unload_pending_) {
UnloadDebugger();
}
@@ -646,6 +646,7 @@
private:
static bool IsDebuggerActive();
+ static void ListenersChanged();
static Mutex* debugger_access_; // Mutex guarding debugger variables.
static Handle<Object> event_listener_; // Global handle to listener.
@@ -654,7 +655,7 @@
static bool is_loading_debugger_; // Are we loading the debugger?
static bool never_unload_debugger_; // Can we unload the debugger?
static v8::Debug::MessageHandler2 message_handler_;
- static bool message_handler_cleared_; // Was message handler cleared?
+ static bool debugger_unload_pending_; // Was message handler cleared?
static v8::Debug::HostDispatchHandler host_dispatch_handler_;
static int host_dispatch_micros_;
Modified: branches/bleeding_edge/test/cctest/test-debug.cc
==============================================================================
--- branches/bleeding_edge/test/cctest/test-debug.cc (original)
+++ branches/bleeding_edge/test/cctest/test-debug.cc Mon May 25 00:51:04
2009
@@ -4693,6 +4693,9 @@
// Two times compile event and two times break event.
CHECK_GT(message_handler_hit_count, 4);
+
+ v8::Debug::SetMessageHandler2(NULL);
+ CheckDebuggerUnloaded();
}
@@ -4739,11 +4742,14 @@
// One time compile event and one time break event.
CHECK_GT(message_handler_hit_count, 2);
+ v8::Debug::SetMessageHandler2(NULL);
+ CheckDebuggerUnloaded();
}
static bool sent_eval = false;
static int break_count = 0;
+static int continue_command_send_count = 0;
// Check that the expected context is the one generating the debug event
// including the case of nested break event.
static void DebugEvalContextCheckMessageHandler(
@@ -4773,11 +4779,13 @@
} else {
// It's a break event caused by the evaluation request above.
SendContinueCommand();
+ continue_command_send_count++;
}
- } else if (message.IsResponse()) {
+ } else if (message.IsResponse() && continue_command_send_count < 2) {
// Response to the evaluation request. We're still on the breakpoint so
// send continue.
SendContinueCommand();
+ continue_command_send_count++;
}
}
@@ -4786,6 +4794,8 @@
// in 'evaluate' debugger request.
TEST(NestedBreakEventContextData) {
v8::HandleScope scope;
+ break_count = 0;
+ message_handler_hit_count = 0;
v8::Debug::SetMessageHandler2(DebugEvalContextCheckMessageHandler);
ExecuteScriptForContextCheck();
@@ -4795,6 +4805,8 @@
// One break from the source and another from the evaluate request.
CHECK_EQ(break_count, 2);
+ v8::Debug::SetMessageHandler2(NULL);
+ CheckDebuggerUnloaded();
}
@@ -4814,6 +4826,7 @@
// Test that scripts collected are reported through the debug event
listener.
TEST(ScriptCollectedEvent) {
break_point_hit_count = 0;
+ script_collected_count = 0;
v8::HandleScope scope;
DebugLocalContext env;
@@ -4858,7 +4871,7 @@
// Test that GetEventContext doesn't fail and return empty handle for
// ScriptCollected events.
TEST(ScriptCollectedEventContext) {
- break_point_hit_count = 0;
+ script_collected_message_count = 0;
v8::HandleScope scope;
{ // Scope for the DebugLocalContext.
@@ -4871,7 +4884,6 @@
// collected afterwards.
Heap::CollectAllGarbage();
- script_collected_count = 0;
v8::Debug::SetMessageHandler2(ScriptCollectedMessageHandler);
{
v8::Script::Compile(v8::String::New("eval('a=1')"))->Run();
@@ -4886,4 +4898,102 @@
CHECK_EQ(2, script_collected_message_count);
v8::Debug::SetMessageHandler2(NULL);
+}
+
+
+// Debug event listener which counts the after compile events.
+int after_compile_message_count = 0;
+static void AfterCompileMessageHandler(const v8::Debug::Message& message) {
+ // Count the number of scripts collected.
+ if (message.IsEvent()) {
+ if (message.GetEvent() == v8::AfterCompile) {
+ after_compile_message_count++;
+ } else if (message.GetEvent() == v8::Break) {
+ SendContinueCommand();
+ }
+ }
+}
+
+
+// Tests that after compile event is sent as many times as there are
scripts
+// compiled.
+TEST(AfterCompileMessageWhenMessageHandlerIsReset) {
+ v8::HandleScope scope;
+ DebugLocalContext env;
+ after_compile_message_count = 0;
+ const char* script = "var a=1";
+
+ v8::Debug::SetMessageHandler2(AfterCompileMessageHandler);
+ v8::Script::Compile(v8::String::New(script))->Run();
+ v8::Debug::SetMessageHandler2(NULL);
+
+ v8::Debug::SetMessageHandler2(AfterCompileMessageHandler);
+ v8::Debug::DebugBreak();
+ v8::Script::Compile(v8::String::New(script))->Run();
+
+ // Setting listener to NULL should cause debugger unload.
+ v8::Debug::SetMessageHandler2(NULL);
+ CheckDebuggerUnloaded();
+
+ // Compilation cache should be disabled when debugger is active.
+ CHECK_EQ(2, after_compile_message_count);
+}
+
+
+// Tests that break event is sent when message handler is reset.
+TEST(BreakMessageWhenMessageHandlerIsReset) {
+ v8::HandleScope scope;
+ DebugLocalContext env;
+ after_compile_message_count = 0;
+ const char* script = "function f() {};";
+
+ v8::Debug::SetMessageHandler2(AfterCompileMessageHandler);
+ v8::Script::Compile(v8::String::New(script))->Run();
+ v8::Debug::SetMessageHandler2(NULL);
+
+ v8::Debug::SetMessageHandler2(AfterCompileMessageHandler);
+ v8::Debug::DebugBreak();
+ v8::Local<v8::Function> f =
+
v8::Local<v8::Function>::Cast(env->Global()->Get(v8::String::New("f")));
+ f->Call(env->Global(), 0, NULL);
+
+ // Setting message handler to NULL should cause debugger unload.
+ v8::Debug::SetMessageHandler2(NULL);
+ CheckDebuggerUnloaded();
+
+ // Compilation cache should be disabled when debugger is active.
+ CHECK_EQ(1, after_compile_message_count);
+}
+
+
+static int exception_event_count = 0;
+static void ExceptionMessageHandler(const v8::Debug::Message& message) {
+ if (message.IsEvent() && message.GetEvent() == v8::Exception) {
+ exception_event_count++;
+ SendContinueCommand();
+ }
+}
+
+
+// Tests that exception event is sent when message handler is reset.
+TEST(ExceptionMessageWhenMessageHandlerIsReset) {
+ v8::HandleScope scope;
+ DebugLocalContext env;
+ exception_event_count = 0;
+ const char* script = "function f() {throw new Error()};";
+
+ v8::Debug::SetMessageHandler2(AfterCompileMessageHandler);
+ v8::Script::Compile(v8::String::New(script))->Run();
+ v8::Debug::SetMessageHandler2(NULL);
+
+ v8::Debug::SetMessageHandler2(ExceptionMessageHandler);
+ v8::Local<v8::Function> f =
+
v8::Local<v8::Function>::Cast(env->Global()->Get(v8::String::New("f")));
+ f->Call(env->Global(), 0, NULL);
+
+ // Setting message handler to NULL should cause debugger unload.
+ v8::Debug::SetMessageHandler2(NULL);
+ CheckDebuggerUnloaded();
+
+ CHECK_EQ(1, exception_event_count);
}
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
