Don't run the second pass of the pending phantom callbacks if the heap has been torn down.

R=jochen@chromium.org
BUG=511204
LOG=y

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

Cr-Commit-Position: refs/heads/master@{#29759}

BUILD.gn

@@ -630,6 +630,8 @@ source_set("v8_base") {
     "src/builtins.cc",
     "src/builtins.h",
     "src/bytecodes-irregexp.h",
+    "src/cancelable-task.cc",
+    "src/cancelable-task.h",
     "src/cached-powers.cc",
     "src/cached-powers.h",
     "src/char-predicates.cc",

src/cancelable-task.cc

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/cancelable-task.h"
+
+#include "src/base/platform/platform.h"
+#include "src/v8.h"
+
+namespace v8 {
+namespace internal {
+
+
+CancelableTask::CancelableTask(Isolate* isolate)
+    : isolate_(isolate), is_cancelled_(false) {
+  isolate->RegisterCancelableTask(this);
+}
+
+
+CancelableTask::~CancelableTask() {
+  if (!is_cancelled_) {
+    isolate_->RemoveCancelableTask(this);
+  }
+}
+
+
+}  // namespace internal
+}  // namespace v8

src/cancelable-task.h

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_CANCELABLE_TASK_H_
+#define V8_CANCELABLE_TASK_H_
+
+#include "include/v8-platform.h"
+#include "src/base/macros.h"
+
+namespace v8 {
+namespace internal {
+
+class Isolate;
+
+class CancelableTask : public Task {
+ public:
+  explicit CancelableTask(Isolate* isolate);
+  ~CancelableTask() override;
+
+  void Cancel() { is_cancelled_ = true; }
+
+  void Run() final {
+    if (!is_cancelled_) {
+      RunInternal();
+    }
+  }
+
+  virtual void RunInternal() = 0;
+
+ protected:
+  Isolate* isolate_;
+
+ private:
+  bool is_cancelled_;
+
+  DISALLOW_COPY_AND_ASSIGN(CancelableTask);
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_CANCELABLE_TASK_H_

src/global-handles.cc

@@ -495,25 +495,23 @@ class GlobalHandles::NodeIterator {
   DISALLOW_COPY_AND_ASSIGN(NodeIterator);
 };
 
-class GlobalHandles::PendingPhantomCallbacksSecondPassTask : public v8::Task {
+class GlobalHandles::PendingPhantomCallbacksSecondPassTask
+    : public v8::internal::CancelableTask {
  public:
   // Takes ownership of the contents of pending_phantom_callbacks, leaving it in
   // the same state it would be after a call to Clear().
   PendingPhantomCallbacksSecondPassTask(
       List<PendingPhantomCallback>* pending_phantom_callbacks, Isolate* isolate)
-      : isolate_(isolate) {
+      : CancelableTask(isolate) {
     pending_phantom_callbacks_.Swap(pending_phantom_callbacks);
   }
 
-  ~PendingPhantomCallbacksSecondPassTask() override {}
-
-  void Run() override {
+  void RunInternal() override {
     InvokeSecondPassPhantomCallbacks(&pending_phantom_callbacks_, isolate_);
   }
 
  private:
   List<PendingPhantomCallback> pending_phantom_callbacks_;
-  Isolate* isolate_;
 
   DISALLOW_COPY_AND_ASSIGN(PendingPhantomCallbacksSecondPassTask);
 };
@@ -845,7 +843,7 @@ int GlobalHandles::DispatchPendingPhantomCallbacks(
     if (synchronous_second_pass) {
       InvokeSecondPassPhantomCallbacks(&pending_phantom_callbacks_, isolate());
     } else {
-      auto* task = new PendingPhantomCallbacksSecondPassTask(
+      auto task = new PendingPhantomCallbacksSecondPassTask(
           &pending_phantom_callbacks_, isolate());
       V8::GetCurrentPlatform()->CallOnForegroundThread(
           reinterpret_cast<v8::Isolate*>(isolate()), task);

src/heap/memory-reducer.cc

@@ -16,9 +16,12 @@ const int MemoryReducer::kLongDelayMs = 5000;
 const int MemoryReducer::kShortDelayMs = 500;
 const int MemoryReducer::kMaxNumberOfGCs = 3;
 
+MemoryReducer::TimerTask::TimerTask(MemoryReducer* memory_reducer)
+    : CancelableTask(memory_reducer->heap()->isolate()),
+      memory_reducer_(memory_reducer) {}
 
-void MemoryReducer::TimerTask::Run() {
-  if (heap_is_torn_down_) return;
+
+void MemoryReducer::TimerTask::RunInternal() {
   Heap* heap = memory_reducer_->heap();
   Event event;
   event.type = kTimer;
@@ -32,10 +35,8 @@ void MemoryReducer::TimerTask::Run() {
 
 
 void MemoryReducer::NotifyTimer(const Event& event) {
-  DCHECK(nullptr != pending_task_);
   DCHECK_EQ(kTimer, event.type);
   DCHECK_EQ(kWait, state_.action);
-  pending_task_ = nullptr;
   state_ = Step(state_, event);
   if (state_.action == kRun) {
     DCHECK(heap()->incremental_marking()->IsStopped());
@@ -170,25 +171,13 @@ void MemoryReducer::ScheduleTimer(double delay_ms) {
   // Leave some room for precision error in task scheduler.
   const double kSlackMs = 100;
   v8::Isolate* isolate = reinterpret_cast<v8::Isolate*>(heap()->isolate());
-  DCHECK(nullptr == pending_task_);
-  pending_task_ = new MemoryReducer::TimerTask(this);
+  auto timer_task = new MemoryReducer::TimerTask(this);
   V8::GetCurrentPlatform()->CallDelayedOnForegroundThread(
-      isolate, pending_task_, (delay_ms + kSlackMs) / 1000.0);
-}
-
-
-void MemoryReducer::ClearTask(v8::Task* task) {
-  if (pending_task_ == task) {
-    pending_task_ = nullptr;
-  }
+      isolate, timer_task, (delay_ms + kSlackMs) / 1000.0);
 }
 
 
 void MemoryReducer::TearDown() {
-  if (pending_task_ != nullptr) {
-    pending_task_->NotifyHeapTearDown();
-    pending_task_ = nullptr;
-  }
   state_ = State(kDone, 0, 0);
 }
 

src/heap/memory-reducer.h

@@ -7,6 +7,7 @@
 
 #include "include/v8-platform.h"
 #include "src/base/macros.h"
+#include "src/cancelable-task.h"
 
 namespace v8 {
 namespace internal {
@@ -101,10 +102,8 @@ class MemoryReducer {
     bool can_start_incremental_gc;
   };
 
-  explicit MemoryReducer(Heap* heap)
-      : heap_(heap), state_(kDone, 0, 0.0), pending_task_(nullptr) {}
+  explicit MemoryReducer(Heap* heap) : heap_(heap), state_(kDone, 0, 0.0) {}
   // Callbacks.
-  void NotifyTimer(const Event& event);
   void NotifyMarkCompact(const Event& event);
   void NotifyContextDisposed(const Event& event);
   void NotifyBackgroundIdleNotification(const Event& event);
@@ -114,7 +113,6 @@ class MemoryReducer {
   // Posts a timer task that will call NotifyTimer after the given delay.
   void ScheduleTimer(double delay_ms);
   void TearDown();
-  void ClearTask(v8::Task* task);
   static const int kLongDelayMs;
   static const int kShortDelayMs;
   static const int kMaxNumberOfGCs;
@@ -122,27 +120,21 @@ class MemoryReducer {
   Heap* heap() { return heap_; }
 
  private:
-  class TimerTask : public v8::Task {
+  class TimerTask : public v8::internal::CancelableTask {
    public:
-    explicit TimerTask(MemoryReducer* memory_reducer)
-        : memory_reducer_(memory_reducer), heap_is_torn_down_(false) {}
-    virtual ~TimerTask() {
-      if (!heap_is_torn_down_) {
-        memory_reducer_->ClearTask(this);
-      }
-    }
-    void NotifyHeapTearDown() { heap_is_torn_down_ = true; }
+    explicit TimerTask(MemoryReducer* memory_reducer);
 
    private:
-    // v8::Task overrides.
-    void Run() override;
+    // v8::internal::CancelableTask overrides.
+    void RunInternal() override;
     MemoryReducer* memory_reducer_;
-    bool heap_is_torn_down_;
     DISALLOW_COPY_AND_ASSIGN(TimerTask);
   };
+
+  void NotifyTimer(const Event& event);
+
   Heap* heap_;
   State state_;
-  TimerTask* pending_task_;
 
   DISALLOW_COPY_AND_ASSIGN(MemoryReducer);
 };

src/isolate.cc

@@ -1904,6 +1904,11 @@ void Isolate::Deinit() {
   delete basic_block_profiler_;
   basic_block_profiler_ = NULL;
 
+  for (CancelableTask* task : cancelable_tasks_) {
+    task->Cancel();
+  }
+  cancelable_tasks_.clear();
+
   heap_.TearDown();
   logger_->TearDown();
 
@@ -2783,6 +2788,18 @@ void Isolate::CheckDetachedContextsAfterGC() {
 }
 
 
+void Isolate::RegisterCancelableTask(CancelableTask* task) {
+  cancelable_tasks_.insert(task);
+}
+
+
+void Isolate::RemoveCancelableTask(CancelableTask* task) {
+  auto removed = cancelable_tasks_.erase(task);
+  USE(removed);
+  DCHECK(removed == 1);
+}
+
+
 bool StackLimitCheck::JsHasOverflowed(uintptr_t gap) const {
   StackGuard* stack_guard = isolate_->stack_guard();
 #ifdef USE_SIMULATOR

src/isolate.h

@@ -6,11 +6,14 @@
 #define V8_ISOLATE_H_
 
 #include <queue>
+#include <set>
+
 #include "include/v8-debug.h"
 #include "src/allocation.h"
 #include "src/assert-scope.h"
 #include "src/base/atomicops.h"
 #include "src/builtins.h"
+#include "src/cancelable-task.h"
 #include "src/contexts.h"
 #include "src/date.h"
 #include "src/execution.h"
@@ -1133,6 +1136,9 @@ class Isolate {
 
   FutexWaitListNode* futex_wait_list_node() { return &futex_wait_list_node_; }
 
+  void RegisterCancelableTask(CancelableTask* task);
+  void RemoveCancelableTask(CancelableTask* task);
+
  protected:
   explicit Isolate(bool enable_serializer);
 
@@ -1368,6 +1374,8 @@ class Isolate {
 
   FutexWaitListNode futex_wait_list_node_;
 
+  std::set<CancelableTask*> cancelable_tasks_;
+
   friend class ExecutionAccess;
   friend class HandleScopeImplementer;
   friend class OptimizingCompileDispatcher;

tools/gyp/v8.gyp

@@ -424,6 +424,8 @@
         '../../src/bytecodes-irregexp.h',
         '../../src/cached-powers.cc',
         '../../src/cached-powers.h',
+        '../../src/cancelable-task.cc',
+        '../../src/cancelable-task.h',
         '../../src/char-predicates.cc',
         '../../src/char-predicates-inl.h',
         '../../src/char-predicates.h',