From 20eeff9ae433374956cd5f2bccf08042734405e2 Mon Sep 17 00:00:00 2001 From: "adamk@chromium.org" Date: Wed, 28 May 2014 18:40:04 +0000 Subject: [PATCH] Allow microtasks to throw exceptions and handle them gracefully If the embedder calls V8::TerminateExecution while we're running microtasks, bail out and clear any pending microtasks. All other exceptions are simply swallowed. No current Blink or V8 microtasks throw, this just ensures something sane happens if another embedder decides to pass a throwing microtask (or if ours unexpectedly throw due to, e.g., stack exhaustion). BUG=371566 LOG=Y R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/294943009 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21574 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 1 + src/isolate.cc | 24 ++++++++++++------- test/cctest/test-api.cc | 37 ++++++++++++++++++++++++++++ test/cctest/test-thread-termination.cc | 44 ++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 8 deletions(-) diff --git a/include/v8.h b/include/v8.h index 656aa35..7af4322 100644 --- a/include/v8.h +++ b/include/v8.h @@ -4380,6 +4380,7 @@ class V8_EXPORT Isolate { /** * Experimental: Runs the Microtask Work Queue until empty + * Any exceptions thrown by microtask callbacks are swallowed. */ void RunMicrotasks(); diff --git a/src/isolate.cc b/src/isolate.cc index ce7e05c..81d9c5f 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2254,11 +2254,11 @@ void Isolate::FireCallCompletedCallback() { if (!handle_scope_implementer()->CallDepthIsZero()) return; if (run_microtasks) RunMicrotasks(); // Fire callbacks. Increase call depth to prevent recursive callbacks. - handle_scope_implementer()->IncrementCallDepth(); + v8::Isolate::SuppressMicrotaskExecutionScope suppress( + reinterpret_cast(this)); for (int i = 0; i < call_completed_callbacks_.length(); i++) { call_completed_callbacks_.at(i)(); } - handle_scope_implementer()->DecrementCallDepth(); } @@ -2288,7 +2288,8 @@ void Isolate::RunMicrotasks() { // ASSERT(handle_scope_implementer()->CallDepthIsZero()); // Increase call depth to prevent recursive callbacks. - handle_scope_implementer()->IncrementCallDepth(); + v8::Isolate::SuppressMicrotaskExecutionScope suppress( + reinterpret_cast(this)); while (pending_microtask_count() > 0) { HandleScope scope(this); @@ -2301,13 +2302,20 @@ void Isolate::RunMicrotasks() { for (int i = 0; i < num_tasks; i++) { HandleScope scope(this); Handle microtask(JSFunction::cast(queue->get(i)), this); - // TODO(adamk): This should ignore/clear exceptions instead of Checking. - Execution::Call(this, microtask, factory()->undefined_value(), - 0, NULL).Check(); + Handle exception; + MaybeHandle result = Execution::TryCall( + microtask, factory()->undefined_value(), 0, NULL, &exception); + // If execution is terminating, just bail out. + if (result.is_null() && + !exception.is_null() && + *exception == heap()->termination_exception()) { + // Clear out any remaining callbacks in the queue. + heap()->set_microtask_queue(heap()->empty_fixed_array()); + set_pending_microtask_count(0); + return; + } } } - - handle_scope_implementer()->DecrementCallDepth(); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index df5938d..3a92f76 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -20777,6 +20777,43 @@ TEST(EnqueueMicrotask) { } +static void MicrotaskExceptionOne( + const v8::FunctionCallbackInfo& info) { + v8::HandleScope scope(info.GetIsolate()); + CompileRun("exception1Calls++;"); + info.GetIsolate()->ThrowException( + v8::Exception::Error(v8_str("first"))); +} + + +static void MicrotaskExceptionTwo( + const v8::FunctionCallbackInfo& info) { + v8::HandleScope scope(info.GetIsolate()); + CompileRun("exception2Calls++;"); + info.GetIsolate()->ThrowException( + v8::Exception::Error(v8_str("second"))); +} + + +TEST(RunMicrotasksIgnoresThrownExceptions) { + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope scope(isolate); + CompileRun( + "var exception1Calls = 0;" + "var exception2Calls = 0;"); + isolate->EnqueueMicrotask( + Function::New(isolate, MicrotaskExceptionOne)); + isolate->EnqueueMicrotask( + Function::New(isolate, MicrotaskExceptionTwo)); + TryCatch try_catch; + CompileRun("1+1;"); + CHECK(!try_catch.HasCaught()); + CHECK_EQ(1, CompileRun("exception1Calls")->Int32Value()); + CHECK_EQ(1, CompileRun("exception2Calls")->Int32Value()); +} + + TEST(SetAutorunMicrotasks) { LocalContext env; v8::HandleScope scope(env->GetIsolate()); diff --git a/test/cctest/test-thread-termination.cc b/test/cctest/test-thread-termination.cc index 569ee95..e949569 100644 --- a/test/cctest/test-thread-termination.cc +++ b/test/cctest/test-thread-termination.cc @@ -358,3 +358,47 @@ TEST(TerminateCancelTerminateFromThreadItself) { // Check that execution completed with correct return value. CHECK(v8::Script::Compile(source)->Run()->Equals(v8_str("completed"))); } + + +void MicrotaskShouldNotRun(const v8::FunctionCallbackInfo& info) { + CHECK(false); +} + + +void MicrotaskLoopForever(const v8::FunctionCallbackInfo& info) { + v8::Isolate* isolate = info.GetIsolate(); + v8::HandleScope scope(isolate); + // Enqueue another should-not-run task to ensure we clean out the queue + // when we terminate. + isolate->EnqueueMicrotask(v8::Function::New(isolate, MicrotaskShouldNotRun)); + CompileRun("terminate(); while (true) { }"); + CHECK(v8::V8::IsExecutionTerminating()); +} + + +TEST(TerminateFromOtherThreadWhileMicrotaskRunning) { + semaphore = new v8::internal::Semaphore(0); + TerminatorThread thread(CcTest::i_isolate()); + thread.Start(); + + v8::Isolate* isolate = CcTest::isolate(); + isolate->SetAutorunMicrotasks(false); + v8::HandleScope scope(isolate); + v8::Handle global = + CreateGlobalTemplate(CcTest::isolate(), Signal, DoLoop); + v8::Handle context = + v8::Context::New(CcTest::isolate(), NULL, global); + v8::Context::Scope context_scope(context); + isolate->EnqueueMicrotask(v8::Function::New(isolate, MicrotaskLoopForever)); + // The second task should never be run because we bail out if we're + // terminating. + isolate->EnqueueMicrotask(v8::Function::New(isolate, MicrotaskShouldNotRun)); + isolate->RunMicrotasks(); + + v8::V8::CancelTerminateExecution(isolate); + isolate->RunMicrotasks(); // should not run MicrotaskShouldNotRun + + thread.Join(); + delete semaphore; + semaphore = NULL; +} -- 2.7.4