From 473d3e1e7100775165cd470ca046069e129cd616 Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Fri, 23 May 2014 08:34:10 +0000 Subject: [PATCH] Make v8::TryCatch able to consume natively thrown exceptions (again). R=yangguo@chromium.org BUG=chromium:362388 TEST=cctest/test-api/TryCatchNative LOG=N Review URL: https://codereview.chromium.org/291393002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21456 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/api.cc | 6 +++++ src/isolate.cc | 15 ++++++++---- src/isolate.h | 2 ++ test/cctest/test-api.cc | 61 +++++++++++++++++++++++++++++++++++++++++++------ 4 files changed, 73 insertions(+), 11 deletions(-) diff --git a/src/api.cc b/src/api.cc index c33365b..66692b8 100644 --- a/src/api.cc +++ b/src/api.cc @@ -1830,6 +1830,12 @@ v8::TryCatch::~TryCatch() { reinterpret_cast(isolate_)->ThrowException(exc); ASSERT(!isolate_->thread_local_top()->rethrowing_message_); } else { + if (HasCaught() && isolate_->has_scheduled_exception()) { + // If an exception was caught but is still scheduled because no API call + // promoted it, then it is canceled to prevent it from being propagated. + // Note that this will not cancel termination exceptions. + isolate_->CancelScheduledExceptionFromTryCatch(this); + } isolate_->UnregisterTryCatchHandler(this); v8::internal::SimulatorStack::UnregisterCTryCatch(); } diff --git a/src/isolate.cc b/src/isolate.cc index 9572046..08948fc 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -908,6 +908,15 @@ void Isolate::RestorePendingMessageFromTryCatch(v8::TryCatch* handler) { } +void Isolate::CancelScheduledExceptionFromTryCatch(v8::TryCatch* handler) { + ASSERT(has_scheduled_exception()); + if (scheduled_exception() == handler->exception_) { + ASSERT(scheduled_exception() != heap()->termination_exception()); + clear_scheduled_exception(); + } +} + + Object* Isolate::PromoteScheduledException() { Object* thrown = scheduled_exception(); clear_scheduled_exception(); @@ -1213,8 +1222,7 @@ void Isolate::ReportPendingMessages() { PropagatePendingExceptionToExternalTryCatch(); HandleScope scope(this); - if (thread_local_top_.pending_exception_ == - heap()->termination_exception()) { + if (thread_local_top_.pending_exception_ == heap()->termination_exception()) { // Do nothing: if needed, the exception has been already propagated to // v8::TryCatch. } else { @@ -1751,8 +1759,7 @@ void Isolate::PropagatePendingExceptionToExternalTryCatch() { if (!external_caught) return; - if (thread_local_top_.pending_exception_ == - heap()->termination_exception()) { + if (thread_local_top_.pending_exception_ == heap()->termination_exception()) { try_catch_handler()->can_continue_ = false; try_catch_handler()->has_terminated_ = true; try_catch_handler()->exception_ = heap()->null_value(); diff --git a/src/isolate.h b/src/isolate.h index 382cb5d..ca2c23e 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -735,6 +735,8 @@ class Isolate { // Re-set pending message, script and positions reported to the TryCatch // back to the TLS for re-use when rethrowing. void RestorePendingMessageFromTryCatch(v8::TryCatch* handler); + // Un-schedule an exception that was caught by a TryCatch handler. + void CancelScheduledExceptionFromTryCatch(v8::TryCatch* handler); void ReportPendingMessages(); // Return pending location if any or unfilled structure. MessageLocation GetMessageLocation(); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index e2f7462..003a511 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -5343,15 +5343,28 @@ THREADED_TEST(TryCatchAndFinally) { } -static void TryCatchNestedHelper(int depth) { +static void TryCatchNested1Helper(int depth) { if (depth > 0) { v8::TryCatch try_catch; try_catch.SetVerbose(true); - TryCatchNestedHelper(depth - 1); + TryCatchNested1Helper(depth - 1); CHECK(try_catch.HasCaught()); try_catch.ReThrow(); } else { - CcTest::isolate()->ThrowException(v8_str("back")); + CcTest::isolate()->ThrowException(v8_str("E1")); + } +} + + +static void TryCatchNested2Helper(int depth) { + if (depth > 0) { + v8::TryCatch try_catch; + try_catch.SetVerbose(true); + TryCatchNested2Helper(depth - 1); + CHECK(try_catch.HasCaught()); + try_catch.ReThrow(); + } else { + CompileRun("throw 'E2';"); } } @@ -5360,10 +5373,22 @@ TEST(TryCatchNested) { v8::V8::Initialize(); LocalContext context; v8::HandleScope scope(context->GetIsolate()); - v8::TryCatch try_catch; - TryCatchNestedHelper(5); - CHECK(try_catch.HasCaught()); - CHECK_EQ(0, strcmp(*v8::String::Utf8Value(try_catch.Exception()), "back")); + + { + // Test nested try-catch with a native throw in the end. + v8::TryCatch try_catch; + TryCatchNested1Helper(5); + CHECK(try_catch.HasCaught()); + CHECK_EQ(0, strcmp(*v8::String::Utf8Value(try_catch.Exception()), "E1")); + } + + { + // Test nested try-catch with a JavaScript throw in the end. + v8::TryCatch try_catch; + TryCatchNested2Helper(5); + CHECK(try_catch.HasCaught()); + CHECK_EQ(0, strcmp(*v8::String::Utf8Value(try_catch.Exception()), "E2")); + } } @@ -5409,6 +5434,28 @@ TEST(TryCatchMixedNesting) { } +void TryCatchNativeHelper(const v8::FunctionCallbackInfo& args) { + ApiTestFuzzer::Fuzz(); + v8::TryCatch try_catch; + args.GetIsolate()->ThrowException(v8_str("boom")); + CHECK(try_catch.HasCaught()); +} + + +TEST(TryCatchNative) { + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope scope(isolate); + v8::V8::Initialize(); + v8::TryCatch try_catch; + Local templ = ObjectTemplate::New(isolate); + templ->Set(v8_str("TryCatchNativeHelper"), + v8::FunctionTemplate::New(isolate, TryCatchNativeHelper)); + LocalContext context(0, templ); + CompileRun("TryCatchNativeHelper();"); + CHECK(!try_catch.HasCaught()); +} + + THREADED_TEST(Equality) { LocalContext context; v8::Isolate* isolate = context->GetIsolate(); -- 2.7.4