Clean up stack guard interrupts.

src/execution.cc

Index: src/execution.cc
diff --git a/src/execution.cc b/src/execution.cc
index b74ef4d62ea6b0b14558fce3810308af42e4bf8a..ee6cb970cb8bd6d852f3e34a681e82d9f3efb686 100644
--- a/src/execution.cc
+++ b/src/execution.cc
@@ -366,190 +366,37 @@ void StackGuard::DisableInterrupts() {
 }
 
 
-bool StackGuard::ShouldPostponeInterrupts() {
+bool StackGuard::CheckInterrupt(int flagbit) {
   ExecutionAccess access(isolate_);
-  return should_postpone_interrupts(access);
+  return thread_local_.interrupt_flags_ & flagbit;
 }
 
 
-bool StackGuard::IsInterrupted() {
+void StackGuard::RequestInterrupt(int flagbit) {
   ExecutionAccess access(isolate_);
-  return (thread_local_.interrupt_flags_ & INTERRUPT) != 0;
-}
-
-
-void StackGuard::Interrupt() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= INTERRUPT;
-  set_interrupt_limits(access);
-}
-
-
-bool StackGuard::IsPreempted() {
-  ExecutionAccess access(isolate_);
-  return thread_local_.interrupt_flags_ & PREEMPT;
-}
-
-
-void StackGuard::Preempt() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= PREEMPT;
+  thread_local_.interrupt_flags_ |= flagbit;
   set_interrupt_limits(access);
 }
 
 
-bool StackGuard::IsTerminateExecution() {
+void StackGuard::ClearInterrupt(int flagbit) {
   ExecutionAccess access(isolate_);
-  return (thread_local_.interrupt_flags_ & TERMINATE) != 0;
-}
-
-
-void StackGuard::CancelTerminateExecution() {
-  ExecutionAccess access(isolate_);
-  Continue(TERMINATE);
-  isolate_->CancelTerminateExecution();
-}
-
-
-void StackGuard::TerminateExecution() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= TERMINATE;
-  set_interrupt_limits(access);
-}
-
-
-bool StackGuard::IsGCRequest() {
-  ExecutionAccess access(isolate_);
-  return (thread_local_.interrupt_flags_ & GC_REQUEST) != 0;
-}
-
-
-void StackGuard::RequestGC() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= GC_REQUEST;
-  if (thread_local_.postpone_interrupts_nesting_ == 0) {
-    thread_local_.jslimit_ = thread_local_.climit_ = kInterruptLimit;
-    isolate_->heap()->SetStackLimits();
-  }
-}
-
-
-bool StackGuard::IsInstallCodeRequest() {
-  ExecutionAccess access(isolate_);
-  return (thread_local_.interrupt_flags_ & INSTALL_CODE) != 0;
-}
-
-
-void StackGuard::RequestInstallCode() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= INSTALL_CODE;
-  if (thread_local_.postpone_interrupts_nesting_ == 0) {
-    thread_local_.jslimit_ = thread_local_.climit_ = kInterruptLimit;
-    isolate_->heap()->SetStackLimits();
-  }
-}
-
-
-bool StackGuard::IsFullDeopt() {
-  ExecutionAccess access(isolate_);
-  return (thread_local_.interrupt_flags_ & FULL_DEOPT) != 0;
-}
-
-
-void StackGuard::FullDeopt() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= FULL_DEOPT;
-  set_interrupt_limits(access);
-}
-
-
-bool StackGuard::IsDeoptMarkedAllocationSites() {
-  ExecutionAccess access(isolate_);
-  return (thread_local_.interrupt_flags_ & DEOPT_MARKED_ALLOCATION_SITES) != 0;
-}
-
-
-void StackGuard::DeoptMarkedAllocationSites() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= DEOPT_MARKED_ALLOCATION_SITES;
-  set_interrupt_limits(access);
-}
-
-
-bool StackGuard::IsDebugBreak() {
-  ExecutionAccess access(isolate_);
-  return thread_local_.interrupt_flags_ & DEBUGBREAK;
-}
-
-
-void StackGuard::DebugBreak() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= DEBUGBREAK;
-  set_interrupt_limits(access);
-}
-
-
-bool StackGuard::IsDebugCommand() {
-  ExecutionAccess access(isolate_);
-  return thread_local_.interrupt_flags_ & DEBUGCOMMAND;
-}
-
-
-void StackGuard::DebugCommand() {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= DEBUGCOMMAND;
-  set_interrupt_limits(access);
-}
-
-
-void StackGuard::Continue(InterruptFlag after_what) {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ &= ~static_cast<int>(after_what);
+  thread_local_.interrupt_flags_ &= ~flagbit;
   if (!should_postpone_interrupts(access) && !has_pending_interrupts(access)) {
     reset_limits(access);
   }
 }
 
 
-void StackGuard::RequestInterrupt(InterruptCallback callback, void* data) {
-  ExecutionAccess access(isolate_);
-  thread_local_.interrupt_flags_ |= API_INTERRUPT;
-  thread_local_.interrupt_callback_ = callback;
-  thread_local_.interrupt_callback_data_ = data;
-  set_interrupt_limits(access);
-}
-
-
-void StackGuard::ClearInterrupt() {
-  thread_local_.interrupt_callback_ = 0;
-  thread_local_.interrupt_callback_data_ = 0;
-  Continue(API_INTERRUPT);
-}
-
-
-bool StackGuard::IsAPIInterrupt() {
-  ExecutionAccess access(isolate_);
-  return thread_local_.interrupt_flags_ & API_INTERRUPT;
-}
-
-
-void StackGuard::InvokeInterruptCallback() {
-  InterruptCallback callback = 0;
-  void* data = 0;
-
-  {
-    ExecutionAccess access(isolate_);
-    callback = thread_local_.interrupt_callback_;
-    data = thread_local_.interrupt_callback_data_;
-    thread_local_.interrupt_callback_ = NULL;
-    thread_local_.interrupt_callback_data_ = NULL;
-  }
-
-  if (callback != NULL) {
-    VMState<EXTERNAL> state(isolate_);
-    HandleScope handle_scope(isolate_);
-    callback(reinterpret_cast<v8::Isolate*>(isolate_), data);
+bool StackGuard::CheckAndClearInterrupt(InterruptFlag flag,
+                                        const ExecutionAccess& lock) {
+  int flagbit = 1 << flag;
+  bool result = (thread_local_.interrupt_flags_ & flagbit);
+  thread_local_.interrupt_flags_ &= ~flagbit;
+  if (!should_postpone_interrupts(lock) && !has_pending_interrupts(lock)) {
+    reset_limits(lock);
   }
+  return result;
 }
 
 
@@ -594,8 +441,6 @@ void StackGuard::ThreadLocal::Clear() {
   nesting_ = 0;
   postpone_interrupts_nesting_ = 0;
   interrupt_flags_ = 0;
-  interrupt_callback_ = NULL;
-  interrupt_callback_data_ = NULL;
 }
 
 
@@ -616,8 +461,6 @@ bool StackGuard::ThreadLocal::Initialize(Isolate* isolate) {
   nesting_ = 0;
   postpone_interrupts_nesting_ = 0;
   interrupt_flags_ = 0;
-  interrupt_callback_ = NULL;
-  interrupt_callback_data_ = NULL;
   return should_set_stack_limits;
 }
 
@@ -834,93 +677,52 @@ Handle<String> Execution::GetStackTraceLine(Handle<Object> recv,
 }
 
 
-static Object* RuntimePreempt(Isolate* isolate) {
-  // Clear the preempt request flag.
-  isolate->stack_guard()->Continue(PREEMPT);
-
-  if (isolate->debug()->InDebugger()) {
-    // If currently in the debugger don't do any actual preemption but record
-    // that preemption occoured while in the debugger.
-    isolate->debug()->PreemptionWhileInDebugger();
-  } else {
-    // Perform preemption.
-    v8::Unlocker unlocker(reinterpret_cast<v8::Isolate*>(isolate));
-    Thread::YieldCPU();
-  }
-
-  return isolate->heap()->undefined_value();
-}
-
-
-Object* Execution::DebugBreakHelper(Isolate* isolate) {
+void Execution::DebugBreakHelper(Isolate* isolate) {
   // Just continue if breaks are disabled.
-  if (isolate->debug()->disable_break()) {
-    return isolate->heap()->undefined_value();
-  }
+  if (isolate->debug()->disable_break()) return;
 
   // Ignore debug break during bootstrapping.
-  if (isolate->bootstrapper()->IsActive()) {
-    return isolate->heap()->undefined_value();
-  }
+  if (isolate->bootstrapper()->IsActive()) return;
 
   // Ignore debug break if debugger is not active.
-  if (!isolate->debugger()->IsDebuggerActive()) {
-    return isolate->heap()->undefined_value();
-  }
+  if (!isolate->debugger()->IsDebuggerActive()) return;
 
   StackLimitCheck check(isolate);
-  if (check.HasOverflowed()) {
-    return isolate->heap()->undefined_value();
-  }
+  if (check.HasOverflowed()) return;
 
-  {
-    JavaScriptFrameIterator it(isolate);
+  { JavaScriptFrameIterator it(isolate);
     ASSERT(!it.done());
     Object* fun = it.frame()->function();
     if (fun && fun->IsJSFunction()) {
       // Don't stop in builtin functions.
-      if (JSFunction::cast(fun)->IsBuiltin()) {
-        return isolate->heap()->undefined_value();
-      }
+      if (JSFunction::cast(fun)->IsBuiltin()) return;
       GlobalObject* global = JSFunction::cast(fun)->context()->global_object();
       // Don't stop in debugger functions.
-      if (isolate->debug()->IsDebugGlobal(global)) {
-        return isolate->heap()->undefined_value();
-      }
+      if (isolate->debug()->IsDebugGlobal(global)) return;
     }
   }
 
   // Collect the break state before clearing the flags.
-  bool debug_command_only =
-      isolate->stack_guard()->IsDebugCommand() &&
-      !isolate->stack_guard()->IsDebugBreak();
-
-  // Clear the debug break request flag.
-  isolate->stack_guard()->Continue(DEBUGBREAK);
+  bool debug_command_only = isolate->stack_guard()->CheckDebugCommand() &&
+                            !isolate->stack_guard()->CheckDebugBreak();
 
-  ProcessDebugMessages(isolate, debug_command_only);
+  isolate->stack_guard()->ClearDebugBreak();
 
-  // Return to continue execution.
-  return isolate->heap()->undefined_value();
+  Execution::ProcessDebugMessages(isolate, debug_command_only);
 }
 
 
 void Execution::ProcessDebugMessages(Isolate* isolate,
                                      bool debug_command_only) {
-  // Clear the debug command request flag.
-  isolate->stack_guard()->Continue(DEBUGCOMMAND);
+  isolate->stack_guard()->ClearDebugCommand();
 
   StackLimitCheck check(isolate);
-  if (check.HasOverflowed()) {
-    return;
-  }
+  if (check.HasOverflowed()) return;
 
   HandleScope scope(isolate);
   // Enter the debugger. Just continue if we fail to enter the debugger.
   EnterDebugger debugger(isolate);
-  if (debugger.FailedToEnter()) {
-    return;
-  }
+  if (debugger.FailedToEnter()) return;
 
   // Notify the debug event listeners. Indicate auto continue if the break was
   // a debug command break.
@@ -929,52 +731,45 @@ void Execution::ProcessDebugMessages(Isolate* isolate,
 }
 
 
-Object* Execution::HandleStackGuardInterrupt(Isolate* isolate) {
-  StackGuard* stack_guard = isolate->stack_guard();
-  if (stack_guard->ShouldPostponeInterrupts()) {
-    return isolate->heap()->undefined_value();
+Object* StackGuard::HandleInterrupts() {
+  ExecutionAccess access(isolate_);

[yurys, 2014/05/29 08:42:07]

Scope of this lock seems too coarse. Do we really need to hold the lock when
handling debug break? This causes a dead lock in blink when we try to terminate
worker which is running a debug loop on pause: v8::V8::TerminateExecution is
called on the main thread while ExecutionAccess lock is held on the worker
thread. Can we revert this part of the change and restore previous behavior
where the lock is only held while interrupt flags are accessed?

+  if (should_postpone_interrupts(access)) {
+    return isolate_->heap()->undefined_value();
   }
 
-  if (stack_guard->IsAPIInterrupt()) {
-    stack_guard->InvokeInterruptCallback();
-    stack_guard->Continue(API_INTERRUPT);
+  if (CheckAndClearInterrupt(API_INTERRUPT, access)) {
+    isolate_->InvokeApiInterruptCallback();
   }
 
-  if (stack_guard->IsGCRequest()) {
-    isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags,
-                                       "StackGuard GC request");
-    stack_guard->Continue(GC_REQUEST);
+  if (CheckAndClearInterrupt(GC_REQUEST, access)) {
+    isolate_->heap()->CollectAllGarbage(Heap::kNoGCFlags, "GC interrupt");
   }
 
-  isolate->counters()->stack_interrupts()->Increment();
-  isolate->counters()->runtime_profiler_ticks()->Increment();
-  if (stack_guard->IsDebugBreak() || stack_guard->IsDebugCommand()) {
-    DebugBreakHelper(isolate);
-  }
-  if (stack_guard->IsPreempted()) RuntimePreempt(isolate);
-  if (stack_guard->IsTerminateExecution()) {
-    stack_guard->Continue(TERMINATE);
-    return isolate->TerminateExecution();
+  if (CheckDebugBreak() || CheckDebugCommand()) {
+    Execution::DebugBreakHelper(isolate_);
   }
-  if (stack_guard->IsInterrupted()) {
-    stack_guard->Continue(INTERRUPT);
-    return isolate->StackOverflow();
+
+  if (CheckAndClearInterrupt(TERMINATE_EXECUTION, access)) {
+    return isolate_->TerminateExecution();
   }
-  if (stack_guard->IsFullDeopt()) {
-    stack_guard->Continue(FULL_DEOPT);
-    Deoptimizer::DeoptimizeAll(isolate);
+
+  if (CheckAndClearInterrupt(FULL_DEOPT, access)) {
+    Deoptimizer::DeoptimizeAll(isolate_);
   }
-  if (stack_guard->IsDeoptMarkedAllocationSites()) {
-    stack_guard->Continue(DEOPT_MARKED_ALLOCATION_SITES);
-    isolate->heap()->DeoptMarkedAllocationSites();
+
+  if (CheckAndClearInterrupt(DEOPT_MARKED_ALLOCATION_SITES, access)) {
+    isolate_->heap()->DeoptMarkedAllocationSites();
   }
-  if (stack_guard->IsInstallCodeRequest()) {
-    ASSERT(isolate->concurrent_recompilation_enabled());
-    stack_guard->Continue(INSTALL_CODE);
-    isolate->optimizing_compiler_thread()->InstallOptimizedFunctions();
+
+  if (CheckAndClearInterrupt(INSTALL_CODE, access)) {
+    ASSERT(isolate_->concurrent_recompilation_enabled());
+    isolate_->optimizing_compiler_thread()->InstallOptimizedFunctions();
   }
-  isolate->runtime_profiler()->OptimizeNow();
-  return isolate->heap()->undefined_value();
+
+  isolate_->counters()->stack_interrupts()->Increment();
+  isolate_->counters()->runtime_profiler_ticks()->Increment();
+  isolate_->runtime_profiler()->OptimizeNow();
+  return isolate_->heap()->undefined_value();
 }
 
 } }  // namespace v8::internal