Implement async AbortAll for the compiler dispatcher

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/isolate.h"
 
 namespace v8 {
@@ skipping 16 matching lines @@
 }
 
 CancelableTaskManager::CancelableTaskManager()
     : task_id_counter_(0), canceled_(false) {}
 
 uint32_t CancelableTaskManager::Register(Cancelable* task) {
   base::LockGuard<base::Mutex> guard(&mutex_);
   uint32_t id = ++task_id_counter_;
   // The loop below is just used when task_id_counter_ overflows.
   while (cancelable_tasks_.count(id) > 0) ++id;
-  CHECK(!canceled_);
-  cancelable_tasks_[id] = task;
+  if (canceled_) {

[marja, 2017/01/04 12:54:43]

Why is this needed now?

Is it because we might now cancel without waiting for all tasks to finish, and
when those tasks keep running, they might schedule new tasks?

(A comment would be good.)

+    CHECK(task->Cancel());

[vogelheim, 2017/01/04 12:58:08]

Why are we sure that Cancel() will always return true in this context?

+  } else {
+    cancelable_tasks_[id] = task;
+  }
   return id;
 }
 
 
 void CancelableTaskManager::RemoveFinishedTask(uint32_t id) {
   base::LockGuard<base::Mutex> guard(&mutex_);
   size_t removed = cancelable_tasks_.erase(id);
   USE(removed);
   DCHECK_NE(0u, removed);
   cancelable_tasks_barrier_.NotifyOne();
@@ skipping 37 matching lines @@
         cancelable_tasks_.erase(current);
       }
     }
     // Wait for already running background tasks.
     if (!cancelable_tasks_.empty()) {
       cancelable_tasks_barrier_.Wait(&mutex_);
     }
   }
 }
 
+CancelableTaskManager::TryAbortResult CancelableTaskManager::Cancel() {
+  // Clean up all cancelable fore- and background tasks. Tasks are canceled on
+  // the way if possible, i.e., if they have not started yet.
+  base::LockGuard<base::Mutex> guard(&mutex_);
+
+  if (cancelable_tasks_.empty()) return kTaskRemoved;
+
+  canceled_ = true;
+
+  bool tasks_running = false;
+
+  for (auto it = cancelable_tasks_.begin(); it != cancelable_tasks_.end();) {
+    auto current = it;
+    // We need to get to the next element before erasing the current.
+    ++it;
+    if (current->second->Cancel()) {
+      cancelable_tasks_.erase(current);
+    } else {
+      tasks_running = true;
+    }
+  }
+
+  canceled_ = false;

[vogelheim, 2017/01/04 12:58:08]

I'm not sure I understang this code... 

Most CancelableTaskmanager calls lock the mutex, so noone should be able to
observe the changes to canceled_. Do those have any effect?


I guess I'm nitpicking/opinionating here, but maybe an easier way to write this
would be to split the loop, in one cancel-loop and one remove-loop. Something
like this:

for (auto it : cancelable_tasks_) it->second->Cancel();
std::remove_if(cancelable_tasks_.begin(), cancelable_tasks.end(),
               [](...whatever...) { return whatever.second->status_ !=
kRunning();});
return cancelable_tasks_.empty() ? ... : ...;

[jochen (gone - plz use gerrit), 2017/01/04 13:18:31]

ah, fair enough. I forgot about the lock..

remove_if doesn't work on maps... so I kept my loop

+
+  return tasks_running ? kTaskRunning : kTaskAborted;

[vogelheim, 2017/01/04 12:58:08]

tasks_running == !cancelable_tasks_.empty() ?
If so, you can drop that variable.

+}
+
 CancelableTask::CancelableTask(Isolate* isolate)
     : CancelableTask(isolate, isolate->cancelable_task_manager()) {}
 
 CancelableTask::CancelableTask(Isolate* isolate, CancelableTaskManager* manager)
     : Cancelable(manager), isolate_(isolate) {}
 
 CancelableIdleTask::CancelableIdleTask(Isolate* isolate)
     : CancelableIdleTask(isolate, isolate->cancelable_task_manager()) {}
 
 CancelableIdleTask::CancelableIdleTask(Isolate* isolate,
                                        CancelableTaskManager* manager)
     : Cancelable(manager), isolate_(isolate) {}
 
 }  // namespace internal
 }  // namespace v8