Implement async AbortAll for the compiler dispatcher

src/cancelable-task.cc

Index: src/cancelable-task.cc
diff --git a/src/cancelable-task.cc b/src/cancelable-task.cc
index b712d98877ce930946bd8ceb4754bf4fe403ef8f..2b941e4fee9864ed5c2eb4ba2c67318629151aa7 100644
--- a/src/cancelable-task.cc
+++ b/src/cancelable-task.cc
@@ -34,8 +34,11 @@ uint32_t CancelableTaskManager::Register(Cancelable* task) {
   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;
 }
 
@@ -93,6 +96,33 @@ void CancelableTaskManager::CancelAndWait() {
   }
 }
 
+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()) {}