Reland "Add mechanism to postpone interrupts selectively."
authoryangguo@chromium.org <yangguo@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 2 Jul 2014 08:05:40 +0000 (08:05 +0000)
committeryangguo@chromium.org <yangguo@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 2 Jul 2014 08:05:40 +0000 (08:05 +0000)
BUG=v8:3408
LOG=N
R=yurys@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22142 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/execution.cc
src/execution.h
src/isolate.cc
src/isolate.h
test/cctest/test-debug.cc
test/cctest/test-thread-termination.cc

index 2a2c90a..3a31c7f 100644 (file)
@@ -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();
@@ -331,36 +329,71 @@ void StackGuard::DisableInterrupts() {
 }
 
 
-bool StackGuard::CheckInterrupt(int flagbit) {
+void StackGuard::PushPostponeInterruptsScope(PostponeInterruptsScope* scope) {
   ExecutionAccess access(isolate_);
-  return thread_local_.interrupt_flags_ & flagbit;
+  // Intercept already requested interrupts.
+  int intercepted = thread_local_.interrupt_flags_ & scope->intercept_mask_;
+  scope->intercepted_flags_ = intercepted;
+  thread_local_.interrupt_flags_ &= ~intercepted;
+  if (!has_pending_interrupts(access)) reset_limits(access);
+  // Add scope to the chain.
+  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_;
+  // Make intercepted interrupts active.
+  ASSERT((thread_local_.interrupt_flags_ & top->intercept_mask_) == 0);
+  thread_local_.interrupt_flags_ |= top->intercepted_flags_;
+  if (has_pending_interrupts(access)) set_interrupt_limits(access);
+  // Remove scope from chain.
+  thread_local_.postpone_interrupts_ = top->prev_;
+}
+
+
+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;
 }
 
@@ -402,8 +435,7 @@ void StackGuard::ThreadLocal::Clear() {
   jslimit_ = kIllegalLimit;
   real_climit_ = kIllegalLimit;
   climit_ = kIllegalLimit;
-  nesting_ = 0;
-  postpone_interrupts_nesting_ = 0;
+  postpone_interrupts_ = NULL;
   interrupt_flags_ = 0;
 }
 
@@ -422,8 +454,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;
 }
@@ -642,13 +673,6 @@ Handle<String> Execution::GetStackTraceLine(Handle<Object> 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");
   }
index 81fbbb8..8d7b693 100644 (file)
@@ -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_;
   };
 
index 1592667..a93f4c0 100644 (file)
@@ -2368,4 +2368,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
index 972a518..a283bd3 100644 (file)
@@ -1425,22 +1425,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;
 };
 
 
index acb3a57..69e0a2e 100644 (file)
@@ -6431,19 +6431,12 @@ static void BreakMessageHandler(const v8::Debug::Message& message) {
   } else if (message.IsEvent() && message.GetEvent() == v8::AfterCompile) {
     i::HandleScope scope(isolate);
 
-    bool is_debug_break = isolate->stack_guard()->CheckDebugBreak();
-    // Force DebugBreak flag while serializer is working.
-    isolate->stack_guard()->RequestDebugBreak();
+    int current_count = break_point_hit_count;
 
     // Force serialization to trigger some internal JS execution.
     message.GetJSON();
 
-    // Restore previous state.
-    if (is_debug_break) {
-      isolate->stack_guard()->RequestDebugBreak();
-    } else {
-      isolate->stack_guard()->ClearDebugBreak();
-    }
+    CHECK_EQ(current_count, break_point_hit_count);
   }
 }
 
index 0eb95f0..06b077d 100644 (file)
@@ -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<v8::ObjectTemplate> global =
+      CreateGlobalTemplate(CcTest::isolate(), TerminateCurrentThread, DoLoop);
+  v8::Handle<v8::Context> 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);
+}