Reland "Trigger exception debug event for promises at the throw site."

R=rossberg@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@21097 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8-debug.h

@@ -20,8 +20,7 @@ enum DebugEvent {
   BeforeCompile = 4,
   AfterCompile  = 5,
   ScriptCollected = 6,
-  PendingExceptionInPromise = 7,
-  BreakForCommand = 8
+  BreakForCommand = 7
 };
 
 

src/debug-debugger.js

@@ -19,8 +19,7 @@ Debug.DebugEvent = { Break: 1,
                      NewFunction: 3,
                      BeforeCompile: 4,
                      AfterCompile: 5,
-                     ScriptCollected: 6,
-                     PendingExceptionInPromise: 7 };
+                     ScriptCollected: 6 };
 
 // Types of exceptions that can be broken upon.
 Debug.ExceptionBreak = { Caught : 0,

src/debug.cc

@@ -37,6 +37,8 @@ Debug::Debug(Isolate* isolate)
       disable_break_(false),
       break_on_exception_(false),
       break_on_uncaught_exception_(false),
+      promise_catch_handlers_(0),
+      promise_getters_(0),
       debug_break_return_(NULL),
       debug_break_slot_(NULL),
       isolate_(isolate) {
@@ -1318,6 +1320,53 @@ bool Debug::IsBreakOnException(ExceptionBreakType type) {
 }
 
 
+void Debug::PromiseHandlePrologue(Handle<JSFunction> promise_getter) {
+  Handle<JSFunction> promise_getter_global = Handle<JSFunction>::cast(
+      isolate_->global_handles()->Create(*promise_getter));
+  StackHandler* handler =
+      StackHandler::FromAddress(Isolate::handler(isolate_->thread_local_top()));
+  promise_getters_.Add(promise_getter_global);
+  promise_catch_handlers_.Add(handler);
+}
+
+
+void Debug::PromiseHandleEpilogue() {
+  if (promise_catch_handlers_.length() == 0) return;
+  promise_catch_handlers_.RemoveLast();
+  Handle<Object> promise_getter = promise_getters_.RemoveLast();
+  isolate_->global_handles()->Destroy(promise_getter.location());
+}
+
+
+Handle<Object> Debug::GetPromiseForUncaughtException() {
+  Handle<Object> undefined = isolate_->factory()->undefined_value();
+  if (promise_getters_.length() == 0) return undefined;
+  Handle<JSFunction> promise_getter = promise_getters_.last();
+  StackHandler* promise_catch = promise_catch_handlers_.last();
+  // Find the top-most try-catch handler.
+  StackHandler* handler = StackHandler::FromAddress(
+      Isolate::handler(isolate_->thread_local_top()));
+  while (handler != NULL && !handler->is_catch()) {
+    handler = handler->next();
+  }
+#ifdef DEBUG
+  // Make sure that our promise catch handler is in the list of handlers,
+  // even if it's not the top-most try-catch handler.
+  StackHandler* temp = handler;
+  while (temp != promise_catch && !temp->is_catch()) {
+    temp = temp->next();
+    CHECK(temp != NULL);
+  }
+#endif  // DEBUG
+
+  if (handler == promise_catch) {
+    return Execution::Call(
+        isolate_, promise_getter, undefined, 0, NULL).ToHandleChecked();
+  }
+  return undefined;
+}
+
+
 void Debug::PrepareStep(StepAction step_action,
                         int step_count,
                         StackFrame::Id frame_id) {
@@ -2647,9 +2696,7 @@ MaybeHandle<Object> Debugger::MakeScriptCollectedEvent(int id) {
 }
 
 
-void Debugger::OnException(Handle<Object> exception,
-                           bool uncaught,
-                           Handle<Object> promise) {
+void Debugger::OnException(Handle<Object> exception, bool uncaught) {
   HandleScope scope(isolate_);
   Debug* debug = isolate_->debug();
 
@@ -2657,6 +2704,9 @@ void Debugger::OnException(Handle<Object> exception,
   if (debug->InDebugger()) return;
   if (!Debugger::EventActive(v8::Exception)) return;
 
+  Handle<Object> promise = debug->GetPromiseForUncaughtException();
+  uncaught |= !promise->IsUndefined();
+
   // Bail out if exception breaks are not active
   if (uncaught) {
     // Uncaught exceptions are reported by either flags.
@@ -2674,10 +2724,6 @@ void Debugger::OnException(Handle<Object> exception,
   // Clear all current stepping setup.
   debug->ClearStepping();
 
-  // Determine event;
-  DebugEvent event = promise->IsUndefined()
-      ? v8::Exception : v8::PendingExceptionInPromise;
-
   // Create the event data object.
   Handle<Object> event_data;
   // Bail out and don't call debugger if exception.
@@ -2687,7 +2733,7 @@ void Debugger::OnException(Handle<Object> exception,
   }
 
   // Process debug event.
-  ProcessDebugEvent(event, Handle<JSObject>::cast(event_data), false);
+  ProcessDebugEvent(v8::Exception, Handle<JSObject>::cast(event_data), false);
   // Return to continue execution from where the exception was thrown.
 }
 
@@ -3169,7 +3215,8 @@ void Debugger::SetMessageHandler(v8::Debug::MessageHandler2 handler) {
 
 
 void Debugger::ListenersChanged() {
-  if (IsDebuggerActive()) {
+  bool active = IsDebuggerActive();
+  if (active) {
     // Disable the compilation cache when the debugger is active.
     isolate_->compilation_cache()->Disable();
     debugger_unload_pending_ = false;

src/debug.h

@@ -235,6 +235,12 @@ class Debug {
   void FloodHandlerWithOneShot();
   void ChangeBreakOnException(ExceptionBreakType type, bool enable);
   bool IsBreakOnException(ExceptionBreakType type);
+
+  void PromiseHandlePrologue(Handle<JSFunction> promise_getter);
+  void PromiseHandleEpilogue();
+  // Returns a promise if it does not have a reject handler.
+  Handle<Object> GetPromiseForUncaughtException();
+
   void PrepareStep(StepAction step_action,
                    int step_count,
                    StackFrame::Id frame_id);
@@ -538,6 +544,14 @@ class Debug {
   bool break_on_exception_;
   bool break_on_uncaught_exception_;
 
+  // When a promise is being resolved, we may want to trigger a debug event for
+  // the case we catch a throw.  For this purpose we remember the try-catch
+  // handler address that would catch the exception.  We also hold onto a
+  // closure that returns a promise if the exception is considered uncaught.
+  // Due to the possibility of reentry we use a list to form a stack.
+  List<StackHandler*> promise_catch_handlers_;
+  List<Handle<JSFunction> > promise_getters_;
+
   // Per-thread data.
   class ThreadLocal {
    public:
@@ -774,9 +788,7 @@ class Debugger {
   MUST_USE_RESULT MaybeHandle<Object> MakeScriptCollectedEvent(int id);
 
   void OnDebugBreak(Handle<Object> break_points_hit, bool auto_continue);
-  void OnException(Handle<Object> exception,
-                   bool uncaught,
-                   Handle<Object> promise = Handle<Object>::null());
+  void OnException(Handle<Object> exception, bool uncaught);
   void OnBeforeCompile(Handle<Script> script);
 
   enum AfterCompileFlags {

src/isolate.cc

@@ -1031,8 +1031,7 @@ void Isolate::DoThrow(Object* exception, MessageLocation* location) {
 
   // Notify debugger of exception.
   if (catchable_by_javascript) {
-    debugger_->OnException(
-        exception_handle, report_exception, factory()->undefined_value());
+    debugger_->OnException(exception_handle, report_exception);
   }
 
   // Generate the message if required.

src/mirror-debugger.js

@@ -1186,7 +1186,7 @@ inherits(PromiseMirror, ObjectMirror);
 
 
 PromiseMirror.prototype.status = function() {
-  var status = %GetPromiseStatus(this.value_);
+  var status = builtins.GetPromiseStatus(this.value_);
   if (status == 0) return "pending";
   if (status == 1) return "resolved";
   return "rejected";
@@ -1194,7 +1194,7 @@ PromiseMirror.prototype.status = function() {
 
 
 PromiseMirror.prototype.promiseValue = function() {
-  return %GetPromiseValue(this.value_);
+  return builtins.GetPromiseValue(this.value_);
 };
 
 

src/promise.js

@@ -35,10 +35,13 @@ function Promise(resolver) {
     throw MakeTypeError('resolver_not_a_function', [resolver]);
   var promise = PromiseInit(this);
   try {
+    %DebugPromiseHandlePrologue(function() { return promise });
     resolver(function(x) { PromiseResolve(promise, x) },
              function(r) { PromiseReject(promise, r) });
   } catch (e) {
     PromiseReject(promise, e);
+  } finally {
+    %DebugPromiseHandleEpilogue();
   }
 }
 
@@ -161,6 +164,11 @@ function PromiseEnqueue(value, tasks) {
 
 function PromiseHandle(value, handler, deferred) {
   try {
+    %DebugPromiseHandlePrologue(
+        function() {
+          var queue = GET_PRIVATE(deferred.promise, promiseOnReject);
+          return (queue && queue.length == 0) ? deferred.promise : UNDEFINED;
+        });
     var result = handler(value);
     if (result === deferred.promise)
       throw MakeTypeError('promise_cyclic', [result]);
@@ -169,21 +177,14 @@ function PromiseHandle(value, handler, deferred) {
     else
       deferred.resolve(result);
   } catch (exception) {
-    var uncaught = false;
-    var reject_queue = GET_PRIVATE(deferred.promise, promiseOnReject);
-    if (reject_queue && reject_queue.length == 0) {
-      // The deferred promise may get a reject handler attached later.
-      // For now, we consider the exception to be (for the moment) uncaught.
-      uncaught = true;
-    }
     try {
+      %DebugPromiseHandlePrologue(function() { return deferred.promise });
       deferred.reject(exception);
-    } catch (e) {
-      // The reject handler can only throw for a custom deferred promise.
-      // We consider the original exception to be uncaught.
-      uncaught = true;
+    } catch (e) { } finally {
+      %DebugPromiseHandleEpilogue();
     }
-    if (uncaught) %DebugPendingExceptionInPromise(exception, deferred.promise);
+  } finally {
+    %DebugPromiseHandleEpilogue();
   }
 }
 
@@ -321,14 +322,6 @@ function GetPromiseStatus(promise) {
   return GET_PRIVATE(promise, promiseStatus);
 }
 
-function GetPromiseOnResolve(promise) {
-  return GET_PRIVATE(promise, promiseOnResolve);
-}
-
-function GetPromiseOnReject(promise) {
-  return GET_PRIVATE(promise, promiseOnReject);
-}
-
 function GetPromiseValue(promise) {
   return GET_PRIVATE(promise, promiseValue);
 }

src/runtime.cc

@@ -5657,13 +5657,22 @@ RUNTIME_FUNCTION(Runtime_DebugPrepareStepInIfStepping) {
 }
 
 
-// Notify the debugger if an expcetion in a promise is not caught (yet).
-RUNTIME_FUNCTION(Runtime_DebugPendingExceptionInPromise) {
-  ASSERT(args.length() == 2);
+// The argument is a closure that is kept until the epilogue is called.
+// On exception, the closure is called, which returns the promise if the
+// exception is considered uncaught, or undefined otherwise.
+RUNTIME_FUNCTION(Runtime_DebugPromiseHandlePrologue) {
+  ASSERT(args.length() == 1);
   HandleScope scope(isolate);
-  CONVERT_ARG_HANDLE_CHECKED(Object, exception, 0);
-  CONVERT_ARG_HANDLE_CHECKED(JSObject, promise, 1);
-  isolate->debugger()->OnException(exception, true, promise);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, promise_getter, 0);
+  isolate->debug()->PromiseHandlePrologue(promise_getter);
+  return isolate->heap()->undefined_value();
+}
+
+
+RUNTIME_FUNCTION(Runtime_DebugPromiseHandleEpilogue) {
+  ASSERT(args.length() == 0);
+  SealHandleScope shs(isolate);
+  isolate->debug()->PromiseHandleEpilogue();
   return isolate->heap()->undefined_value();
 }
 

src/runtime.h

@@ -75,7 +75,8 @@ namespace internal {
   F(SetInlineBuiltinFlag, 1, 1) \
   F(StoreArrayLiteralElement, 5, 1) \
   F(DebugPrepareStepInIfStepping, 1, 1) \
-  F(DebugPendingExceptionInPromise, 2, 1) \
+  F(DebugPromiseHandlePrologue, 1, 1) \
+  F(DebugPromiseHandleEpilogue, 0, 1) \
   F(FlattenString, 1, 1) \
   F(LoadMutableDouble, 2, 1) \
   F(TryMigrateInstance, 1, 1) \

test/mjsunit/es6/debug-promises-caught-all.js

@@ -6,8 +6,7 @@
 
 // Test debug events when we listen to all exceptions and
 // there is a catch handler for the exception thrown in a Promise.
-// Expectation:
-//  - only the normal Exception debug event is triggered.
+// We expect a normal Exception debug event to be triggered.
 
 Debug = debug.Debug;
 
@@ -35,10 +34,10 @@ function listener(event, exec_state, event_data, data) {
     // Ignore exceptions during startup in stress runs.
     if (step >= 1) return;
     assertEquals(["resolve", "end main", "throw"], log);
-    assertTrue(event != Debug.DebugEvent.PendingExceptionInPromise);
     if (event == Debug.DebugEvent.Exception) {
       assertEquals("caught", event_data.exception().message);
       assertEquals(undefined, event_data.promise());
+      assertFalse(event_data.uncaught());
       step++;
     }
   } catch (e) {

test/mjsunit/es6/debug-promises-caught-late.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: --harmony-promises --expose-debug-as debug
+
+// Test debug events when we only listen to uncaught exceptions, the Promise
+// throws, and a catch handler is installed right before throwing.
+// We expect no debug event to be triggered.
+
+Debug = debug.Debug;
+
+var p = new Promise(function(resolve, reject) {
+  resolve();
+});
+
+var q = p.chain(
+  function() {
+    q.catch(function(e) {
+      assertEquals("caught", e.message);
+    });
+    throw new Error("caught");
+  });
+
+function listener(event, exec_state, event_data, data) {
+  try {
+    assertTrue(event != Debug.DebugEvent.Exception);
+  } catch (e) {
+    // Signal a failure with exit code 1.  This is necessary since the
+    // debugger swallows exceptions and we expect the chained function
+    // and this listener to be executed after the main script is finished.
+    print("Unexpected exception: " + e + "\n" + e.stack);
+    quit(1);
+  }
+}
+
+Debug.setBreakOnUncaughtException();
+Debug.setListener(listener);

test/mjsunit/es6/debug-promises-caught-uncaught.js

@@ -6,8 +6,7 @@
 
 // Test debug events when we only listen to uncaught exceptions and
 // there is a catch handler for the exception thrown in a Promise.
-// Expectation:
-//  - no debug event is triggered.
+// We expect no debug event to be triggered.
 
 Debug = debug.Debug;
 
@@ -28,7 +27,6 @@ q.catch(
 function listener(event, exec_state, event_data, data) {
   try {
     assertTrue(event != Debug.DebugEvent.Exception);
-    assertTrue(event != Debug.DebugEvent.PendingExceptionInPromise);
   } catch (e) {
     // Signal a failure with exit code 1.  This is necessary since the
     // debugger swallows exceptions and we expect the chained function

test/mjsunit/es6/debug-promises-reentry.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: --harmony-promises --expose-debug-as debug
+
+// Test reentry of special try catch for Promises.
+
+Debug = debug.Debug;
+
+Debug.setBreakOnUncaughtException();
+Debug.setListener(function(event, exec_state, event_data, data) { });
+
+var p = new Promise(function(resolve, reject) { resolve(); });
+var q = p.chain(function() {
+  new Promise(function(resolve, reject) { resolve(); });
+});

test/mjsunit/es6/debug-promises-throw-in-constructor.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: --harmony-promises --expose-debug-as debug
+
+// Test debug events when we only listen to uncaught exceptions and
+// an exception is thrown in the the Promise constructor.
+// We expect an Exception debug event with a promise to be triggered.
+
+Debug = debug.Debug;
+
+var step = 0;
+var exception = null;
+
+function listener(event, exec_state, event_data, data) {
+  try {
+    // Ignore exceptions during startup in stress runs.
+    if (step >= 1) return;
+    if (event == Debug.DebugEvent.Exception) {
+      assertEquals(0, step);
+      assertEquals("uncaught", event_data.exception().message);
+      assertTrue(event_data.promise() instanceof Promise);
+      assertTrue(event_data.uncaught());
+      // Assert that the debug event is triggered at the throw site.
+      assertTrue(exec_state.frame(0).sourceLineText().indexOf("// event") > 0);
+      step++;
+    }
+  } catch (e) {
+    // Signal a failure with exit code 1.  This is necessary since the
+    // debugger swallows exceptions and we expect the chained function
+    // and this listener to be executed after the main script is finished.
+    print("Unexpected exception: " + e + "\n" + e.stack);
+    exception = e;
+  }
+}
+
+Debug.setBreakOnUncaughtException();
+Debug.setListener(listener);
+
+var p = new Promise(function(resolve, reject) {
+  throw new Error("uncaught");  // event
+});
+
+assertEquals(1, step);
+assertNull(exception);

test/mjsunit/es6/debug-promises-throw-in-reject.js

@@ -6,7 +6,7 @@
 
 // Test debug events when an exception is thrown inside a Promise, which is
 // caught by a custom promise, which throws a new exception in its reject
-// handler.  We expect a PendingExceptionInPromise event to be triggered.
+// handler.  We expect an Exception debug event with a promise to be triggered.
 
 Debug = debug.Debug;
 
@@ -21,7 +21,7 @@ var p = new Promise(function(resolve, reject) {
 function MyPromise(resolver) {
   var reject = function() {
     log.push("throw reject");
-    throw new Error("reject");
+    throw new Error("reject");  // event
   };
   var resolve = function() { };
   log.push("construct");
@@ -39,12 +39,12 @@ var q = p.chain(
 
 function listener(event, exec_state, event_data, data) {
   try {
-    if (event == Debug.DebugEvent.PendingExceptionInPromise) {
+    if (event == Debug.DebugEvent.Exception) {
       assertEquals(["resolve", "construct", "end main",
                     "throw caught", "throw reject"], log);
-      assertEquals("caught", event_data.exception().message);
-    } else if (event == Debug.DebugEvent.Exception) {
-      assertUnreachable();
+      assertEquals("reject", event_data.exception().message);
+      assertEquals(q, event_data.promise());
+      assertTrue(exec_state.frame(0).sourceLineText().indexOf('// event') > 0);
     }
   } catch (e) {
     // Signal a failure with exit code 1.  This is necessary since the

test/mjsunit/es6/debug-promises-uncaught-all.js

@@ -6,10 +6,7 @@
 
 // Test debug events when we listen to all exceptions and
 // there is a catch handler for the exception thrown in a Promise.
-// Expectation:
-//  - the normal Exception debug event is triggered.
-//  - the PendingExceptionInPromise debug event is triggered afterwards,
-//    with the same exception object.
+// We expect an Exception debug event with a promise to be triggered.
 
 Debug = debug.Debug;
 
@@ -25,28 +22,24 @@ var p = new Promise(function(resolve, reject) {
 var q = p.chain(
   function() {
     log.push("throw");
-    throw new Error("uncaught");
+    throw new Error("uncaught");  // event
   });
 
 function listener(event, exec_state, event_data, data) {
   try {
     // Ignore exceptions during startup in stress runs.
-    if (step > 1) return;
+    if (step >= 1) return;
     assertEquals(["resolve", "end main", "throw"], log);
     if (event == Debug.DebugEvent.Exception) {
       assertEquals(0, step);
-      exception = event_data.exception();
-      assertEquals(undefined, event_data.promise());
-    } else if (event == Debug.DebugEvent.PendingExceptionInPromise) {
-      assertEquals(1, step);
-      assertEquals(exception, event_data.exception());
-      assertEquals("uncaught", exception.message);
+      assertEquals("uncaught", event_data.exception().message);
       assertTrue(event_data.promise() instanceof Promise);
+      assertEquals(q, event_data.promise());
       assertTrue(event_data.uncaught());
-    } else {
-      return;
+      // Assert that the debug event is triggered at the throw site.
+      assertTrue(exec_state.frame(0).sourceLineText().indexOf("// event") > 0);
+      step++;
     }
-    step++;
   } catch (e) {
     // Signal a failure with exit code 1.  This is necessary since the
     // debugger swallows exceptions and we expect the chained function

test/mjsunit/es6/debug-promises-uncaught-uncaught.js

@@ -6,8 +6,7 @@
 
 // Test debug events when we only listen to uncaught exceptions and
 // there is a catch handler for the exception thrown in a Promise.
-// Expectation:
-//  - only the PendingExceptionInPromise debug event is triggered.
+// We expect an Exception debug event with a promise to be triggered.
 
 Debug = debug.Debug;
 
@@ -22,7 +21,7 @@ var p = new Promise(function(resolve, reject) {
 var q = p.chain(
   function() {
     log.push("throw");
-    throw new Error("uncaught");
+    throw new Error("uncaught");  // event
   });
 
 function listener(event, exec_state, event_data, data) {
@@ -31,12 +30,13 @@ function listener(event, exec_state, event_data, data) {
     if (step >= 1) return;
     assertEquals(["resolve", "end main", "throw"], log);
     if (event == Debug.DebugEvent.Exception) {
-      assertUnreachable();
-    } else if (event == Debug.DebugEvent.PendingExceptionInPromise) {
       assertEquals(0, step);
       assertEquals("uncaught", event_data.exception().message);
       assertTrue(event_data.promise() instanceof Promise);
+      assertEquals(q, event_data.promise());
       assertTrue(event_data.uncaught());
+      // Assert that the debug event is triggered at the throw site.
+      assertTrue(exec_state.frame(0).sourceLineText().indexOf("// event") > 0);
       step++;
     }
   } catch (e) {

test/mjsunit/es6/debug-promises-undefined-reject.js

@@ -6,7 +6,7 @@
 
 // Test debug events when an exception is thrown inside a Promise, which is
 // caught by a custom promise, which has no reject handler.
-// We expect a PendingExceptionInPromise event to be triggered.
+// We expect an Exception event with a promise to be triggered.
 
 Debug = debug.Debug;
 
@@ -31,16 +31,16 @@ p.constructor = MyPromise;
 var q = p.chain(
   function() {
     log.push("throw caught");
-    throw new Error("caught");
+    throw new Error("caught");  // event
   });
 
 function listener(event, exec_state, event_data, data) {
   try {
-    if (event == Debug.DebugEvent.PendingExceptionInPromise) {
+    if (event == Debug.DebugEvent.Exception) {
       assertEquals(["resolve", "construct", "end main", "throw caught"], log);
-      assertEquals("caught", event_data.exception().message);
-    } else if (event == Debug.DebugEvent.Exception) {
-      assertUnreachable();
+      assertEquals("undefined is not a function",
+                   event_data.exception().message);
+      assertEquals(q, event_data.promise());
     }
   } catch (e) {
     // Signal a failure with exit code 1.  This is necessary since the