From 326bc2a53383e69bbd3e00d8a3def7438d1fb6d4 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 30 Jun 2014 06:27:20 +0000 Subject: [PATCH] Add mechanism to postpone interrupts selectively. BUG=v8:3408 LOG=N R=yurys@chromium.org Review URL: https://codereview.chromium.org/359723005 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22073 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/execution.cc | 69 +++++++++++++++++++++------------- src/execution.h | 65 +++++++++++++++----------------- src/isolate.cc | 11 ++++++ src/isolate.h | 27 ++++++++----- test/cctest/test-thread-termination.cc | 56 +++++++++++++++++++++++++++ 5 files changed, 157 insertions(+), 71 deletions(-) diff --git a/src/execution.cc b/src/execution.cc index 2766e76..5cb5531 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -20,8 +20,6 @@ StackGuard::StackGuard() void StackGuard::set_interrupt_limits(const ExecutionAccess& lock) { ASSERT(isolate_ != NULL); - // Ignore attempts to interrupt when interrupts are postponed. - if (should_postpone_interrupts(lock)) return; thread_local_.jslimit_ = kInterruptLimit; thread_local_.climit_ = kInterruptLimit; isolate_->heap()->SetStackLimits(); @@ -337,36 +335,62 @@ void StackGuard::DisableInterrupts() { } -bool StackGuard::CheckInterrupt(int flagbit) { +void StackGuard::PushPostponeInterruptsScope(PostponeInterruptsScope* scope) { ExecutionAccess access(isolate_); - return thread_local_.interrupt_flags_ & flagbit; + scope->prev_ = thread_local_.postpone_interrupts_; + thread_local_.postpone_interrupts_ = scope; } -void StackGuard::RequestInterrupt(int flagbit) { +void StackGuard::PopPostponeInterruptsScope() { ExecutionAccess access(isolate_); - thread_local_.interrupt_flags_ |= flagbit; + PostponeInterruptsScope* top = thread_local_.postpone_interrupts_; + thread_local_.interrupt_flags_ |= top->intercepted_flags_; + thread_local_.postpone_interrupts_ = top->prev_; + if (has_pending_interrupts(access)) set_interrupt_limits(access); +} + + +bool StackGuard::CheckInterrupt(InterruptFlag flag) { + ExecutionAccess access(isolate_); + return thread_local_.interrupt_flags_ & flag; +} + + +void StackGuard::RequestInterrupt(InterruptFlag flag) { + ExecutionAccess access(isolate_); + // Check the chain of PostponeInterruptsScopes for interception. + if (thread_local_.postpone_interrupts_ && + thread_local_.postpone_interrupts_->Intercept(flag)) { + return; + } + + // Not intercepted. Set as active interrupt flag. + thread_local_.interrupt_flags_ |= flag; set_interrupt_limits(access); } -void StackGuard::ClearInterrupt(int flagbit) { +void StackGuard::ClearInterrupt(InterruptFlag flag) { ExecutionAccess access(isolate_); - thread_local_.interrupt_flags_ &= ~flagbit; - if (!should_postpone_interrupts(access) && !has_pending_interrupts(access)) { - reset_limits(access); + // Clear the interrupt flag from the chain of PostponeInterruptsScopes. + for (PostponeInterruptsScope* current = thread_local_.postpone_interrupts_; + current != NULL; + current = current->prev_) { + current->intercepted_flags_ &= ~flag; } + + // Clear the interrupt flag from the active interrupt flags. + thread_local_.interrupt_flags_ &= ~flag; + if (!has_pending_interrupts(access)) reset_limits(access); } bool StackGuard::CheckAndClearInterrupt(InterruptFlag flag) { ExecutionAccess access(isolate_); - int flagbit = 1 << flag; - bool result = (thread_local_.interrupt_flags_ & flagbit); - thread_local_.interrupt_flags_ &= ~flagbit; - if (!should_postpone_interrupts(access) && !has_pending_interrupts(access)) { - reset_limits(access); - } + bool result = (thread_local_.interrupt_flags_ & flag); + thread_local_.interrupt_flags_ &= ~flag; + if (!has_pending_interrupts(access)) reset_limits(access); return result; } @@ -408,8 +432,7 @@ void StackGuard::ThreadLocal::Clear() { jslimit_ = kIllegalLimit; real_climit_ = kIllegalLimit; climit_ = kIllegalLimit; - nesting_ = 0; - postpone_interrupts_nesting_ = 0; + postpone_interrupts_ = NULL; interrupt_flags_ = 0; } @@ -428,8 +451,7 @@ bool StackGuard::ThreadLocal::Initialize(Isolate* isolate) { climit_ = limit; should_set_stack_limits = true; } - nesting_ = 0; - postpone_interrupts_nesting_ = 0; + postpone_interrupts_ = NULL; interrupt_flags_ = 0; return should_set_stack_limits; } @@ -648,13 +670,6 @@ Handle Execution::GetStackTraceLine(Handle recv, Object* StackGuard::HandleInterrupts() { - { - ExecutionAccess access(isolate_); - if (should_postpone_interrupts(access)) { - return isolate_->heap()->undefined_value(); - } - } - if (CheckAndClearInterrupt(GC_REQUEST)) { isolate_->heap()->CollectAllGarbage(Heap::kNoGCFlags, "GC interrupt"); } diff --git a/src/execution.h b/src/execution.h index 81fbbb8..8d7b693 100644 --- a/src/execution.h +++ b/src/execution.h @@ -122,6 +122,7 @@ class Execution V8_FINAL : public AllStatic { class ExecutionAccess; +class PostponeInterruptsScope; // StackGuard contains the handling of the limits that are used to limit the @@ -145,22 +146,32 @@ class StackGuard V8_FINAL { // it has been set up. void ClearThread(const ExecutionAccess& lock); -#define INTERRUPT_LIST(V) \ - V(DEBUGBREAK, DebugBreak) \ - V(DEBUGCOMMAND, DebugCommand) \ - V(TERMINATE_EXECUTION, TerminateExecution) \ - V(GC_REQUEST, GC) \ - V(INSTALL_CODE, InstallCode) \ - V(API_INTERRUPT, ApiInterrupt) \ - V(DEOPT_MARKED_ALLOCATION_SITES, DeoptMarkedAllocationSites) - -#define V(NAME, Name) \ - inline bool Check##Name() { return CheckInterrupt(1 << NAME); } \ - inline void Request##Name() { RequestInterrupt(1 << NAME); } \ - inline void Clear##Name() { ClearInterrupt(1 << NAME); } +#define INTERRUPT_LIST(V) \ + V(DEBUGBREAK, DebugBreak, 0) \ + V(DEBUGCOMMAND, DebugCommand, 1) \ + V(TERMINATE_EXECUTION, TerminateExecution, 2) \ + V(GC_REQUEST, GC, 3) \ + V(INSTALL_CODE, InstallCode, 4) \ + V(API_INTERRUPT, ApiInterrupt, 5) \ + V(DEOPT_MARKED_ALLOCATION_SITES, DeoptMarkedAllocationSites, 6) + +#define V(NAME, Name, id) \ + inline bool Check##Name() { return CheckInterrupt(NAME); } \ + inline void Request##Name() { RequestInterrupt(NAME); } \ + inline void Clear##Name() { ClearInterrupt(NAME); } INTERRUPT_LIST(V) #undef V + // Flag used to set the interrupt causes. + enum InterruptFlag { + #define V(NAME, Name, id) NAME = (1 << id), + INTERRUPT_LIST(V) + #undef V + #define V(NAME, Name, id) NAME | + ALL_INTERRUPTS = INTERRUPT_LIST(V) 0 + #undef V + }; + // This provides an asynchronous read of the stack limits for the current // thread. There are no locks protecting this, but it is assumed that you // have the global V8 lock if you are using multiple V8 threads. @@ -190,33 +201,17 @@ class StackGuard V8_FINAL { private: StackGuard(); -// Flag used to set the interrupt causes. -enum InterruptFlag { -#define V(NAME, Name) NAME, - INTERRUPT_LIST(V) -#undef V - NUMBER_OF_INTERRUPTS -}; - - bool CheckInterrupt(int flagbit); - void RequestInterrupt(int flagbit); - void ClearInterrupt(int flagbit); + bool CheckInterrupt(InterruptFlag flag); + void RequestInterrupt(InterruptFlag flag); + void ClearInterrupt(InterruptFlag flag); bool CheckAndClearInterrupt(InterruptFlag flag); // You should hold the ExecutionAccess lock when calling this method. bool has_pending_interrupts(const ExecutionAccess& lock) { - // Sanity check: We shouldn't be asking about pending interrupts - // unless we're not postponing them anymore. - ASSERT(!should_postpone_interrupts(lock)); return thread_local_.interrupt_flags_ != 0; } // You should hold the ExecutionAccess lock when calling this method. - bool should_postpone_interrupts(const ExecutionAccess& lock) { - return thread_local_.postpone_interrupts_nesting_ > 0; - } - - // You should hold the ExecutionAccess lock when calling this method. inline void set_interrupt_limits(const ExecutionAccess& lock); // Reset limits to actual values. For example after handling interrupt. @@ -235,6 +230,9 @@ enum InterruptFlag { static const uintptr_t kIllegalLimit = 0xfffffff8; #endif + void PushPostponeInterruptsScope(PostponeInterruptsScope* scope); + void PopPostponeInterruptsScope(); + class ThreadLocal V8_FINAL { public: ThreadLocal() { Clear(); } @@ -259,8 +257,7 @@ enum InterruptFlag { uintptr_t real_climit_; // Actual C++ stack limit set for the VM. uintptr_t climit_; - int nesting_; - int postpone_interrupts_nesting_; + PostponeInterruptsScope* postpone_interrupts_; int interrupt_flags_; }; diff --git a/src/isolate.cc b/src/isolate.cc index 9bffca2..af23752 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2371,4 +2371,15 @@ bool StackLimitCheck::JsHasOverflowed() const { } +bool PostponeInterruptsScope::Intercept(StackGuard::InterruptFlag flag) { + // First check whether the previous scope intercepts. + if (prev_ && prev_->Intercept(flag)) return true; + // Then check whether this scope intercepts. + if ((flag & intercept_mask_)) { + intercepted_flags_ |= flag; + return true; + } + return false; +} + } } // namespace v8::internal diff --git a/src/isolate.h b/src/isolate.h index ed2ed33..e2ab3fa 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -1420,22 +1420,29 @@ class StackLimitCheck BASE_EMBEDDED { // account. class PostponeInterruptsScope BASE_EMBEDDED { public: - explicit PostponeInterruptsScope(Isolate* isolate) - : stack_guard_(isolate->stack_guard()), isolate_(isolate) { - ExecutionAccess access(isolate_); - stack_guard_->thread_local_.postpone_interrupts_nesting_++; - stack_guard_->DisableInterrupts(); + PostponeInterruptsScope(Isolate* isolate, + int intercept_mask = StackGuard::ALL_INTERRUPTS) + : stack_guard_(isolate->stack_guard()), + intercept_mask_(intercept_mask), + intercepted_flags_(0) { + stack_guard_->PushPostponeInterruptsScope(this); } ~PostponeInterruptsScope() { - ExecutionAccess access(isolate_); - if (--stack_guard_->thread_local_.postpone_interrupts_nesting_ == 0) { - stack_guard_->EnableInterrupts(); - } + stack_guard_->PopPostponeInterruptsScope(); } + + // Find the bottom-most scope that intercepts this interrupt. + // Return whether the interrupt has been intercepted. + bool Intercept(StackGuard::InterruptFlag flag); + private: StackGuard* stack_guard_; - Isolate* isolate_; + int intercept_mask_; + int intercepted_flags_; + PostponeInterruptsScope* prev_; + + friend class StackGuard; }; diff --git a/test/cctest/test-thread-termination.cc b/test/cctest/test-thread-termination.cc index 9c3401f..2f419ed 100644 --- a/test/cctest/test-thread-termination.cc +++ b/test/cctest/test-thread-termination.cc @@ -403,3 +403,59 @@ TEST(TerminateFromOtherThreadWhileMicrotaskRunning) { delete semaphore; semaphore = NULL; } + + +static int callback_counter = 0; + + +static void CounterCallback(v8::Isolate* isolate, void* data) { + callback_counter++; +} + + +TEST(PostponeTerminateException) { + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope scope(isolate); + v8::Handle global = + CreateGlobalTemplate(CcTest::isolate(), TerminateCurrentThread, DoLoop); + v8::Handle context = + v8::Context::New(CcTest::isolate(), NULL, global); + v8::Context::Scope context_scope(context); + + v8::TryCatch try_catch; + static const char* terminate_and_loop = + "terminate(); for (var i = 0; i < 10000; i++);"; + + { // Postpone terminate execution interrupts. + i::PostponeInterruptsScope p1(CcTest::i_isolate(), + i::StackGuard::TERMINATE_EXECUTION) ; + + // API interrupts should still be triggered. + CcTest::isolate()->RequestInterrupt(&CounterCallback, NULL); + CHECK_EQ(0, callback_counter); + CompileRun(terminate_and_loop); + CHECK(!try_catch.HasTerminated()); + CHECK_EQ(1, callback_counter); + + { // Postpone API interrupts as well. + i::PostponeInterruptsScope p2(CcTest::i_isolate(), + i::StackGuard::API_INTERRUPT); + + // None of the two interrupts should trigger. + CcTest::isolate()->RequestInterrupt(&CounterCallback, NULL); + CompileRun(terminate_and_loop); + CHECK(!try_catch.HasTerminated()); + CHECK_EQ(1, callback_counter); + } + + // Now the previously requested API interrupt should trigger. + CompileRun(terminate_and_loop); + CHECK(!try_catch.HasTerminated()); + CHECK_EQ(2, callback_counter); + } + + // Now the previously requested terminate execution interrupt should trigger. + CompileRun("for (var i = 0; i < 10000; i++);"); + CHECK(try_catch.HasTerminated()); + CHECK_EQ(2, callback_counter); +} -- 2.7.4