From bfc0d11981e9f717ab774cbeb7acb8b637033db2 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Mon, 5 Oct 2015 13:13:18 -0700 Subject: [PATCH] deps: backport 1ee712a from V8 upstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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@{#31111} Ref: #3036 PR-URL: https://github.com/nodejs/node/pull/3481 Reviewed-By: targos - Michaƫl Zasso --- deps/v8/include/v8.h | 13 +++++++++++++ deps/v8/src/api.cc | 7 +++++++ deps/v8/src/isolate.cc | 29 ++++++++++++++++++++++------- deps/v8/src/isolate.h | 6 ++++++ deps/v8/test/cctest/test-api.cc | 31 +++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 7 deletions(-) diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index 395354341120de..25142731b16770 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -5377,6 +5377,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. diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc index cfc170ee8e1d3f..fd9477e101557c 100644 --- a/deps/v8/src/api.cc +++ b/deps/v8/src/api.cc @@ -7105,6 +7105,13 @@ void Isolate::Exit() { } +void Isolate::SetAbortOnUncaughtExceptionCallback( + AbortOnUncaughtExceptionCallback callback) { + i::Isolate* isolate = reinterpret_cast(this); + isolate->SetAbortOnUncaughtExceptionCallback(callback); +} + + Isolate::DisallowJavascriptExecutionScope::DisallowJavascriptExecutionScope( Isolate* isolate, Isolate::DisallowJavascriptExecutionScope::OnFailure on_failure) diff --git a/deps/v8/src/isolate.cc b/deps/v8/src/isolate.cc index 3fff6b2ef7a491..481cb42f05fb71 100644 --- a/deps/v8/src/isolate.cc +++ b/deps/v8/src/isolate.cc @@ -1008,13 +1008,21 @@ Object* Isolate::Throw(Object* exception, MessageLocation* location) { Handle 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(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); @@ -1602,6 +1610,12 @@ void Isolate::SetCaptureStackTraceForUncaughtExceptions( } +void Isolate::SetAbortOnUncaughtExceptionCallback( + v8::Isolate::AbortOnUncaughtExceptionCallback callback) { + abort_on_uncaught_exception_callback_ = callback; +} + + Handle Isolate::native_context() { return handle(context()->native_context()); } @@ -1770,7 +1784,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 lock_guard(thread_data_table_mutex_.Pointer()); CHECK(thread_data_table_); diff --git a/deps/v8/src/isolate.h b/deps/v8/src/isolate.h index 035b4b363c2c0c..e77dcd705e0021 100644 --- a/deps/v8/src/isolate.h +++ b/deps/v8/src/isolate.h @@ -655,6 +655,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, @@ -1325,6 +1328,9 @@ class Isolate { std::set cancelable_tasks_; + v8::Isolate::AbortOnUncaughtExceptionCallback + abort_on_uncaught_exception_callback_; + friend class ExecutionAccess; friend class HandleScopeImplementer; friend class OptimizingCompileDispatcher; diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index ab44af7f934aae..93cdce62075fd6 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -21863,3 +21863,34 @@ TEST(EstimatedContextSize) { LocalContext env; CHECK(50000 < env->EstimatedSize()); } + + +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 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 global_object = env->Global(); + v8::Local foo = + v8::Local::Cast(global_object->Get(v8_str("boom"))); + + foo->Call(global_object, 0, NULL); + + CHECK_EQ(1, nb_uncaught_exception_callback_calls); +}