Fix disabling all break points from within the debug event callback.

BUG=chromium:432493
LOG=Y

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

Cr-Commit-Position: refs/heads/master@{#25400}

src/debug.cc

@@ -40,6 +40,7 @@ Debug::Debug(Isolate* isolate)
       live_edit_enabled_(true),  // TODO(yangguo): set to false by default.
       has_break_points_(false),
       break_disabled_(false),
+      in_debug_event_listener_(false),
       break_on_exception_(false),
       break_on_uncaught_exception_(false),
       script_cache_(NULL),
@@ -872,7 +873,7 @@ void Debug::Break(Arguments args, JavaScriptFrame* frame) {
   LiveEdit::InitializeThreadLocal(this);
 
   // Just continue if breaks are disabled or debugger cannot be loaded.
-  if (break_disabled_) return;
+  if (break_disabled()) return;
 
   // Enter the debugger.
   DebugScope debug_scope(this);
@@ -2815,7 +2816,8 @@ void Debug::CallEventCallback(v8::DebugEvent event,
                               Handle<Object> exec_state,
                               Handle<Object> event_data,
                               v8::Debug::ClientData* client_data) {
-  DisableBreak no_break(this, true);
+  bool previous = in_debug_event_listener_;
+  in_debug_event_listener_ = true;
   if (event_listener_->IsForeign()) {
     // Invoke the C debug event listener.
     v8::Debug::EventCallback callback =
@@ -2839,6 +2841,7 @@ void Debug::CallEventCallback(v8::DebugEvent event,
     Execution::TryCall(Handle<JSFunction>::cast(event_listener_),
                        global, arraysize(argv), argv);
   }
+  in_debug_event_listener_ = previous;
 }
 
 
@@ -3092,7 +3095,7 @@ void Debug::HandleDebugBreak() {
   // Ignore debug break during bootstrapping.
   if (isolate_->bootstrapper()->IsActive()) return;
   // Just continue if breaks are disabled.
-  if (break_disabled_) return;
+  if (break_disabled()) return;
   // Ignore debug break if debugger is not active.
   if (!is_active()) return;
 

src/debug.h

@@ -515,6 +515,9 @@ class Debug {
   // Check whether there are commands in the command queue.
   inline bool has_commands() const { return !command_queue_.IsEmpty(); }
   inline bool ignore_events() const { return is_suppressed_ || !is_active_; }
+  inline bool break_disabled() const {
+    return break_disabled_ || in_debug_event_listener_;
+  }
 
   void OnException(Handle<Object> exception, bool uncaught,
                    Handle<Object> promise);
@@ -592,6 +595,7 @@ class Debug {
   bool live_edit_enabled_;
   bool has_break_points_;
   bool break_disabled_;
+  bool in_debug_event_listener_;
   bool break_on_exception_;
   bool break_on_uncaught_exception_;
 
@@ -702,14 +706,21 @@ class DebugScope BASE_EMBEDDED {
 class DisableBreak BASE_EMBEDDED {
  public:
   explicit DisableBreak(Debug* debug, bool disable_break)
-    : debug_(debug), old_state_(debug->break_disabled_) {
+      : debug_(debug),
+        previous_break_disabled_(debug->break_disabled_),
+        previous_in_debug_event_listener_(debug->in_debug_event_listener_) {
     debug_->break_disabled_ = disable_break;
+    debug_->in_debug_event_listener_ = disable_break;
+  }
+  ~DisableBreak() {
+    debug_->break_disabled_ = previous_break_disabled_;
+    debug_->in_debug_event_listener_ = previous_in_debug_event_listener_;
   }
-  ~DisableBreak() { debug_->break_disabled_ = old_state_; }
 
  private:
   Debug* debug_;
-  bool old_state_;
+  bool previous_break_disabled_;
+  bool previous_in_debug_event_listener_;
   DISALLOW_COPY_AND_ASSIGN(DisableBreak);
 };
 

test/mjsunit/regress/regress-crbug-432493.js

+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug
+
+function f() {
+  var a = 1;
+  var b = 2;
+  return a + b;
+}
+
+var exception = null;
+var break_count = 0;
+var throw_count = 0;
+
+function listener(event, exec_state, event_data, data) {
+  try {
+    if (event == Debug.DebugEvent.Break) {
+      break_count++;
+      // Disable all breakpoints from within the debug event callback.
+      Debug.debuggerFlags().breakPointsActive.setValue(false);
+    } else if (event = Debug.DebugEvent.Exception) {
+      throw_count++;
+      // Enable all breakpoints from within the debug event callback.
+      Debug.debuggerFlags().breakPointsActive.setValue(true);
+    }
+  } catch (e) {
+    exception = e;
+  }
+}
+
+Debug = debug.Debug;
+
+Debug.setListener(listener);
+Debug.setBreakOnException();
+Debug.setBreakPoint(f, 2);
+
+f();
+f();
+
+assertEquals(1, break_count);
+assertEquals(0, throw_count);
+
+// Trigger exception event.
+try { throw 1; } catch (e) {}
+
+f();
+f();
+
+Debug.setListener(null);
+Debug.clearBreakOnException();
+Debug.debuggerFlags().breakPointsActive.setValue(true);
+
+assertEquals(2, break_count);
+assertEquals(1, throw_count);
+assertNull(exception);