Skip to content

Commit

Permalink
deps: backport 1ee712a from V8 upstream
Browse files Browse the repository at this point in the history
Backport 1ee712ab8687e5f4dec93d45da068d37d28feb8b from V8 upstream.

Original commit message:

  Add SetAbortOnUncaughtExceptionCallback API

  The --abort-on-uncaught-exception command line switch makes
  Isolate::Throw abort if the error being thrown cannot be caught by a
  try/catch block.

  Embedders may want to use other mechanisms than try/catch blocks to
  handle uncaught exceptions. For instance, Node.js has "domain" objects
  that have error handlers that can handle uncaught exception like
  following:

  var d = domain.create();

  d.on('error', function onError(err) {
    console.log('Handling error');
  });

  d.run(function() {
    throw new Error("boom");
  });

  These error handlers are called by isolates' message listeners.

  If --abort-on-uncaught-exception is *not* used, the isolate's
  message listener will be called, which will in turn call the domain's
  error handler. The process will output 'Handling error' and will exit
  successfully (not due to an uncaught exception). This is the behavior
  that Node.js users expect.

  However, if --abort-on-uncaught-exception is used and when throwing an
  error within a domain that has an error handler, the process will abort
  and the domain's error handler will not be called. This is not the
  behavior that Node.js users expect.

  Having a SetAbortOnUncaughtExceptionCallback API allows embedders to
  determine when it's not appropriate to abort and instead handle the
  exception via the isolate's message listener.

  In the example above, Node.js would set a custom callback with
  SetAbortOnUncaughtExceptionCallback that would be implemented as
  following (the sample code has been simplified to remove what's not
  relevant to this change):

  bool ShouldAbortOnUncaughtException(Isolate* isolate) {
    return !IsDomainActive();
  }

  Now when --abort-on-uncaught-exception is used, Isolate::Throw would
  call that callback and determine that it should not abort if a domain
  with an error handler is active. Instead, the isolate's message listener
  would be called and the error would be handled by the domain's error
  handler.

  I believe this can also be useful for other embedders.

  BUG=

  R=bmeurer@chromium.org

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

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

PR: nodejs#3036
PR-URL: nodejs#3036
Reviewed-By: Ben Noordhuis <ben@strongloop.com>
  • Loading branch information
Julien Gilli committed Oct 6, 2015
1 parent 8593b3e commit 49dec1a
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 7 deletions.
13 changes: 13 additions & 0 deletions deps/v8/include/v8.h
Expand Up @@ -5380,6 +5380,19 @@ class V8_EXPORT Isolate {
*/
static Isolate* GetCurrent();

/**
* Custom callback used by embedders to help V8 determine if it should abort
* when it throws and no internal handler is predicted to catch the
* exception. If --abort-on-uncaught-exception is used on the command line,
* then V8 will abort if either:
* - no custom callback is set.
* - the custom callback set returns true.
* Otherwise, the custom callback will not be called and V8 will not abort.
*/
typedef bool (*AbortOnUncaughtExceptionCallback)(Isolate*);
void SetAbortOnUncaughtExceptionCallback(
AbortOnUncaughtExceptionCallback callback);

/**
* Methods below this point require holding a lock (using Locker) in
* a multi-threaded environment.
Expand Down
7 changes: 7 additions & 0 deletions deps/v8/src/api.cc
Expand Up @@ -7176,6 +7176,13 @@ void Isolate::Exit() {
}


void Isolate::SetAbortOnUncaughtExceptionCallback(
AbortOnUncaughtExceptionCallback callback) {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
isolate->SetAbortOnUncaughtExceptionCallback(callback);
}


Isolate::DisallowJavascriptExecutionScope::DisallowJavascriptExecutionScope(
Isolate* isolate,
Isolate::DisallowJavascriptExecutionScope::OnFailure on_failure)
Expand Down
29 changes: 22 additions & 7 deletions deps/v8/src/isolate.cc
Expand Up @@ -1013,13 +1013,21 @@ Object* Isolate::Throw(Object* exception, MessageLocation* location) {
Handle<Object> message_obj = CreateMessage(exception_handle, location);
thread_local_top()->pending_message_obj_ = *message_obj;

// If the abort-on-uncaught-exception flag is specified, abort on any
// exception not caught by JavaScript, even when an external handler is
// present. This flag is intended for use by JavaScript developers, so
// print a user-friendly stack trace (not an internal one).
// For any exception not caught by JavaScript, even when an external
// handler is present:
// If the abort-on-uncaught-exception flag is specified, and if the
// embedder didn't specify a custom uncaught exception callback,
// or if the custom callback determined that V8 should abort, then
// abort.
if (FLAG_abort_on_uncaught_exception &&
PredictExceptionCatcher() != CAUGHT_BY_JAVASCRIPT) {
FLAG_abort_on_uncaught_exception = false; // Prevent endless recursion.
PredictExceptionCatcher() != CAUGHT_BY_JAVASCRIPT &&
(!abort_on_uncaught_exception_callback_ ||
abort_on_uncaught_exception_callback_(
reinterpret_cast<v8::Isolate*>(this)))) {
// Prevent endless recursion.
FLAG_abort_on_uncaught_exception = false;
// This flag is intended for use by JavaScript developers, so
// print a user-friendly stack trace (not an internal one).
PrintF(stderr, "%s\n\nFROM\n",
MessageHandler::GetLocalizedMessage(this, message_obj).get());
PrintCurrentStackTrace(stderr);
Expand Down Expand Up @@ -1612,6 +1620,12 @@ void Isolate::SetCaptureStackTraceForUncaughtExceptions(
}


void Isolate::SetAbortOnUncaughtExceptionCallback(
v8::Isolate::AbortOnUncaughtExceptionCallback callback) {
abort_on_uncaught_exception_callback_ = callback;
}


Handle<Context> Isolate::native_context() {
return handle(context()->native_context());
}
Expand Down Expand Up @@ -1782,7 +1796,8 @@ Isolate::Isolate(bool enable_serializer)
next_unique_sfi_id_(0),
#endif
use_counter_callback_(NULL),
basic_block_profiler_(NULL) {
basic_block_profiler_(NULL),
abort_on_uncaught_exception_callback_(NULL) {
{
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
CHECK(thread_data_table_);
Expand Down
6 changes: 6 additions & 0 deletions deps/v8/src/isolate.h
Expand Up @@ -690,6 +690,9 @@ class Isolate {
int frame_limit,
StackTrace::StackTraceOptions options);

void SetAbortOnUncaughtExceptionCallback(
v8::Isolate::AbortOnUncaughtExceptionCallback callback);

enum PrintStackMode { kPrintStackConcise, kPrintStackVerbose };
void PrintCurrentStackTrace(FILE* out);
void PrintStack(StringStream* accumulator,
Expand Down Expand Up @@ -1363,6 +1366,9 @@ class Isolate {

v8::ArrayBuffer::Allocator* array_buffer_allocator_;

v8::Isolate::AbortOnUncaughtExceptionCallback
abort_on_uncaught_exception_callback_;

friend class ExecutionAccess;
friend class HandleScopeImplementer;
friend class OptimizingCompileDispatcher;
Expand Down
31 changes: 31 additions & 0 deletions deps/v8/test/cctest/test-api.cc
Expand Up @@ -21880,3 +21880,34 @@ TEST(CompatibleReceiverCheckOnCachedICHandler) {
"result;\n",
0);
}


static int nb_uncaught_exception_callback_calls = 0;


bool NoAbortOnUncaughtException(v8::Isolate* isolate) {
++nb_uncaught_exception_callback_calls;
return false;
}


TEST(AbortOnUncaughtExceptionNoAbort) {
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope handle_scope(isolate);
v8::Handle<v8::ObjectTemplate> global_template =
v8::ObjectTemplate::New(isolate);
LocalContext env(NULL, global_template);

i::FLAG_abort_on_uncaught_exception = true;
isolate->SetAbortOnUncaughtExceptionCallback(NoAbortOnUncaughtException);

CompileRun("function boom() { throw new Error(\"boom\") }");

v8::Local<v8::Object> global_object = env->Global();
v8::Local<v8::Function> foo =
v8::Local<v8::Function>::Cast(global_object->Get(v8_str("boom")));

foo->Call(global_object, 0, NULL);

CHECK_EQ(1, nb_uncaught_exception_callback_calls);
}

0 comments on commit 49dec1a

Please sign in to comment.