[runtime] implemented SafeForInterruptsScope

This CL introduced SafeForInterruptsScope. This scope overrides
outer PostponeInterruptsScopes:
- reschedule postponed interrupts if needed,
- allow requesting new interrupts.
As soon as scope removed interrupts are posponed if needed.

This scope will be:
- used to allow inspector to interrupt and terminate
  DebugeEvaluate::Local,
- exposed with new flag on Isolate to implement SafeForTerminationScope
  in blink.

R=yangguo@chromium.org

Bug: chromium:820640
Change-Id: I15befc10c2cee393d1e3be48cecb31ee14dae638
Reviewed-on: https://chromium-review.googlesource.com/1022969
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52743}

src/execution.cc

@@ -315,29 +315,54 @@ void StackGuard::DisableInterrupts() {
   reset_limits(access);
 }
 
-
-void StackGuard::PushPostponeInterruptsScope(PostponeInterruptsScope* scope) {
+void StackGuard::PushInterruptsScope(InterruptsScope* scope) {
   ExecutionAccess access(isolate_);
+  if (scope->mode_ == InterruptsScope::kPostponeInterrupts) {
     // Intercept already requested interrupts.
     int intercepted = thread_local_.interrupt_flags_ & scope->intercept_mask_;
     scope->intercepted_flags_ = intercepted;
     thread_local_.interrupt_flags_ &= ~intercepted;
+  } else {
+    DCHECK_EQ(scope->mode_, InterruptsScope::kRunInterrupts);
+    // Restore postponed interrupts.
+    int restored_flags = 0;
+    for (InterruptsScope* current = thread_local_.interrupt_scopes_;
+         current != nullptr; current = current->prev_) {
+      restored_flags |= (current->intercepted_flags_ & scope->intercept_mask_);
+      current->intercepted_flags_ &= ~scope->intercept_mask_;
+    }
+    thread_local_.interrupt_flags_ |= restored_flags;
+  }
   if (!has_pending_interrupts(access)) reset_limits(access);
   // Add scope to the chain.
-  scope->prev_ = thread_local_.postpone_interrupts_;
-  thread_local_.postpone_interrupts_ = scope;
+  scope->prev_ = thread_local_.interrupt_scopes_;
+  thread_local_.interrupt_scopes_ = scope;
 }
 
-
-void StackGuard::PopPostponeInterruptsScope() {
+void StackGuard::PopInterruptsScope() {
   ExecutionAccess access(isolate_);
-  PostponeInterruptsScope* top = thread_local_.postpone_interrupts_;
+  InterruptsScope* top = thread_local_.interrupt_scopes_;
+  if (top->mode_ == InterruptsScope::kPostponeInterrupts) {
     // Make intercepted interrupts active.
     DCHECK_EQ(thread_local_.interrupt_flags_ & top->intercept_mask_, 0);
     thread_local_.interrupt_flags_ |= top->intercepted_flags_;
+  } else {
+    DCHECK_EQ(top->mode_, InterruptsScope::kRunInterrupts);
+    // Postpone existing interupts if needed.
+    if (top->prev_) {
+      for (int interrupt = 1; interrupt < ALL_INTERRUPTS;
+           interrupt = interrupt << 1) {
+        InterruptFlag flag = static_cast<InterruptFlag>(interrupt);
+        if ((thread_local_.interrupt_flags_ & flag) &&
+            top->prev_->Intercept(flag)) {
+          thread_local_.interrupt_flags_ &= ~flag;
+        }
+      }
+    }
+  }
   if (has_pending_interrupts(access)) set_interrupt_limits(access);
   // Remove scope from chain.
-  thread_local_.postpone_interrupts_ = top->prev_;
+  thread_local_.interrupt_scopes_ = top->prev_;
 }
 
 
@@ -349,9 +374,9 @@ bool StackGuard::CheckInterrupt(InterruptFlag 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)) {
+  // Check the chain of InterruptsScope for interception.
+  if (thread_local_.interrupt_scopes_ &&
+      thread_local_.interrupt_scopes_->Intercept(flag)) {
     return;
   }
 
@@ -366,8 +391,8 @@ void StackGuard::RequestInterrupt(InterruptFlag flag) {
 
 void StackGuard::ClearInterrupt(InterruptFlag flag) {
   ExecutionAccess access(isolate_);
-  // Clear the interrupt flag from the chain of PostponeInterruptsScopes.
-  for (PostponeInterruptsScope* current = thread_local_.postpone_interrupts_;
+  // Clear the interrupt flag from the chain of InterruptsScope.
+  for (InterruptsScope* current = thread_local_.interrupt_scopes_;
        current != nullptr; current = current->prev_) {
     current->intercepted_flags_ &= ~flag;
   }
@@ -424,7 +449,7 @@ void StackGuard::ThreadLocal::Clear() {
   set_jslimit(kIllegalLimit);
   real_climit_ = kIllegalLimit;
   set_climit(kIllegalLimit);
-  postpone_interrupts_ = nullptr;
+  interrupt_scopes_ = nullptr;
   interrupt_flags_ = 0;
 }
 
@@ -441,7 +466,7 @@ bool StackGuard::ThreadLocal::Initialize(Isolate* isolate) {
     set_climit(limit);
     should_set_stack_limits = true;
   }
-  postpone_interrupts_ = nullptr;
+  interrupt_scopes_ = nullptr;
   interrupt_flags_ = 0;
   return should_set_stack_limits;
 }

src/execution.h

@@ -60,8 +60,7 @@ class Execution final : public AllStatic {
 
 
 class ExecutionAccess;
-class PostponeInterruptsScope;
-
+class InterruptsScope;
 
 // StackGuard contains the handling of the limits that are used to limit the
 // number of nested invocations of JavaScript and the stack size used in each
@@ -170,8 +169,8 @@ class V8_EXPORT_PRIVATE StackGuard final {
   static const uintptr_t kIllegalLimit = 0xfffffff8;
 #endif
 
-  void PushPostponeInterruptsScope(PostponeInterruptsScope* scope);
-  void PopPostponeInterruptsScope();
+  void PushInterruptsScope(InterruptsScope* scope);
+  void PopInterruptsScope();
 
   class ThreadLocal final {
    public:
@@ -215,7 +214,7 @@ class V8_EXPORT_PRIVATE StackGuard final {
                                  static_cast<base::AtomicWord>(limit));
     }
 
-    PostponeInterruptsScope* postpone_interrupts_;
+    InterruptsScope* interrupt_scopes_;
     int interrupt_flags_;
   };
 
@@ -226,7 +225,7 @@ class V8_EXPORT_PRIVATE StackGuard final {
 
   friend class Isolate;
   friend class StackLimitCheck;
-  friend class PostponeInterruptsScope;
+  friend class InterruptsScope;
 
   DISALLOW_COPY_AND_ASSIGN(StackGuard);
 };

src/isolate.cc

@@ -4144,17 +4144,24 @@ AssertNoContextChange::AssertNoContextChange(Isolate* isolate)
     : isolate_(isolate), context_(isolate->context(), isolate) {}
 #endif  // DEBUG
 
-
-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;
+bool InterruptsScope::Intercept(StackGuard::InterruptFlag flag) {
+  InterruptsScope* last_postpone_scope = nullptr;
+  for (InterruptsScope* current = this; current; current = current->prev_) {
+    // We only consider scopes related to passed flag.
+    if (!(current->intercept_mask_ & flag)) continue;
+    if (current->mode_ == kRunInterrupts) {
+      // If innermost scope is kRunInterrupts scope, prevent interrupt from
+      // beeing prevented.
+      if (!last_postpone_scope) return false;
+    } else {
+      DCHECK_EQ(current->mode_, kPostponeInterrupts);
+      last_postpone_scope = current;
     }
-  return false;
+  }
+  // If there is no postpone scope for passed flag then we should not inrecept.
+  if (!last_postpone_scope) return false;
+  last_postpone_scope->intercepted_flags_ |= flag;
+  return true;
 }
-
 }  // namespace internal
 }  // namespace v8

src/isolate.h

@@ -1823,37 +1823,62 @@ class StackLimitCheck BASE_EMBEDDED {
     }                                      \
   } while (false)
 
-// Support for temporarily postponing interrupts. When the outermost
-// postpone scope is left the interrupts will be re-enabled and any
-// interrupts that occurred while in the scope will be taken into
-// account.
-class PostponeInterruptsScope BASE_EMBEDDED {
+// Scope intercepts only interrupt which is part of its interrupt_mask and does
+// not affect other interrupts.
+class InterruptsScope {
  public:
-  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);
-  }
+  enum Mode { kPostponeInterrupts, kRunInterrupts };
 
-  ~PostponeInterruptsScope() {
-    stack_guard_->PopPostponeInterruptsScope();
-  }
+  virtual ~InterruptsScope() { stack_guard_->PopInterruptsScope(); }
 
-  // Find the bottom-most scope that intercepts this interrupt.
+  // Find the scope that intercepts this interrupt.
+  // It may be outermost PostponeInterruptsScope or innermost
+  // SafeForInterruptsScope if any.
   // Return whether the interrupt has been intercepted.
   bool Intercept(StackGuard::InterruptFlag flag);
 
+ protected:
+  InterruptsScope(Isolate* isolate, int intercept_mask, Mode mode)
+      : stack_guard_(isolate->stack_guard()),
+        intercept_mask_(intercept_mask),
+        intercepted_flags_(0),
+        mode_(mode) {
+    stack_guard_->PushInterruptsScope(this);
+  }
+
  private:
   StackGuard* stack_guard_;
   int intercept_mask_;
   int intercepted_flags_;
-  PostponeInterruptsScope* prev_;
+  Mode mode_;
+  InterruptsScope* prev_;
 
   friend class StackGuard;
 };
 
+// Support for temporarily postponing interrupts. When the outermost
+// postpone scope is left the interrupts will be re-enabled and any
+// interrupts that occurred while in the scope will be taken into
+// account.
+class PostponeInterruptsScope : public InterruptsScope {
+ public:
+  PostponeInterruptsScope(Isolate* isolate,
+                          int intercept_mask = StackGuard::ALL_INTERRUPTS)
+      : InterruptsScope(isolate, intercept_mask,
+                        InterruptsScope::kPostponeInterrupts) {}
+  virtual ~PostponeInterruptsScope() = default;
+};
+
+// Support for overriding PostponeInterruptsScope. Interrupt is not ignored if
+// innermost scope is SafeForInterruptsScope ignoring any outer
+// PostponeInterruptsScopes.
+class SafeForInterruptsScope : public InterruptsScope {
+ public:
+  SafeForInterruptsScope(Isolate* isolate, int intercept_mask)
+      : InterruptsScope(isolate, intercept_mask,
+                        InterruptsScope::kRunInterrupts) {}
+  virtual ~SafeForInterruptsScope() = default;
+};
 
 class CodeTracer final : public Malloced {
  public:

test/cctest/test-thread-termination.cc

@@ -506,6 +506,103 @@ TEST(PostponeTerminateException) {
   CHECK_EQ(2, callback_counter);
 }
 
+static void AssertTerminatedCodeRun(v8::Isolate* isolate) {
+  v8::TryCatch try_catch(isolate);
+  CompileRun("for (var i = 0; i < 10000; i++);");
+  CHECK(try_catch.HasTerminated());
+}
+
+static void AssertFinishedCodeRun(v8::Isolate* isolate) {
+  v8::TryCatch try_catch(isolate);
+  CompileRun("for (var i = 0; i < 10000; i++);");
+  CHECK(!try_catch.HasTerminated());
+}
+
+TEST(SafeForTerminateException) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Context> context = v8::Context::New(CcTest::isolate());
+  v8::Context::Scope context_scope(context);
+
+  {  // Checks safe for termination scope.
+    i::PostponeInterruptsScope p1(CcTest::i_isolate(),
+                                  i::StackGuard::TERMINATE_EXECUTION);
+    isolate->TerminateExecution();
+    AssertFinishedCodeRun(isolate);
+    {
+      i::SafeForInterruptsScope p2(CcTest::i_isolate(),
+                                   i::StackGuard::TERMINATE_EXECUTION);
+      AssertTerminatedCodeRun(isolate);
+      AssertFinishedCodeRun(isolate);
+      isolate->TerminateExecution();
+    }
+    AssertFinishedCodeRun(isolate);
+    isolate->CancelTerminateExecution();
+  }
+
+  isolate->TerminateExecution();
+  {  // no scope -> postpone
+    i::PostponeInterruptsScope p1(CcTest::i_isolate(),
+                                  i::StackGuard::TERMINATE_EXECUTION);
+    AssertFinishedCodeRun(isolate);
+    {  // postpone -> postpone
+      i::PostponeInterruptsScope p2(CcTest::i_isolate(),
+                                    i::StackGuard::TERMINATE_EXECUTION);
+      AssertFinishedCodeRun(isolate);
+
+      {  // postpone -> safe
+        i::SafeForInterruptsScope p3(CcTest::i_isolate(),
+                                     i::StackGuard::TERMINATE_EXECUTION);
+        AssertTerminatedCodeRun(isolate);
+        isolate->TerminateExecution();
+
+        {  // safe -> safe
+          i::SafeForInterruptsScope p4(CcTest::i_isolate(),
+                                       i::StackGuard::TERMINATE_EXECUTION);
+          AssertTerminatedCodeRun(isolate);
+          isolate->TerminateExecution();
+
+          {  // safe -> postpone
+            i::PostponeInterruptsScope p5(CcTest::i_isolate(),
+                                          i::StackGuard::TERMINATE_EXECUTION);
+            AssertFinishedCodeRun(isolate);
+          }  // postpone -> safe
+
+          AssertTerminatedCodeRun(isolate);
+          isolate->TerminateExecution();
+        }  // safe -> safe
+
+        AssertTerminatedCodeRun(isolate);
+        isolate->TerminateExecution();
+      }  // safe -> postpone
+
+      AssertFinishedCodeRun(isolate);
+    }  // postpone -> postpone
+
+    AssertFinishedCodeRun(isolate);
+  }  // postpone -> no scope
+  AssertTerminatedCodeRun(isolate);
+
+  isolate->TerminateExecution();
+  {  // no scope -> safe
+    i::SafeForInterruptsScope p1(CcTest::i_isolate(),
+                                 i::StackGuard::TERMINATE_EXECUTION);
+    AssertTerminatedCodeRun(isolate);
+  }  // safe -> no scope
+  AssertFinishedCodeRun(isolate);
+
+  {  // no scope -> postpone
+    i::PostponeInterruptsScope p1(CcTest::i_isolate(),
+                                  i::StackGuard::TERMINATE_EXECUTION);
+    isolate->TerminateExecution();
+    {  // postpone -> safe
+      i::SafeForInterruptsScope p2(CcTest::i_isolate(),
+                                   i::StackGuard::TERMINATE_EXECUTION);
+      AssertTerminatedCodeRun(isolate);
+    }  // safe -> postpone
+  }    // postpone -> no scope
+  AssertFinishedCodeRun(isolate);
+}
 
 TEST(ErrorObjectAfterTermination) {
   v8::Isolate* isolate = CcTest::isolate();