TaskScheduler: Don't delete Tasks in the scope of a Sequence's lock.

base/task_scheduler/sequence.cc

Index: base/task_scheduler/sequence.cc
diff --git a/base/task_scheduler/sequence.cc b/base/task_scheduler/sequence.cc
index 4ecb60568cb88a6c013a3fb23d94c629fa22135e..20a5b26ed612570d6cd6df5283b599163c9f143b 100644
--- a/base/task_scheduler/sequence.cc
+++ b/base/task_scheduler/sequence.cc
@@ -36,16 +36,27 @@ const Task* Sequence::PeekTask() const {
 }
 
 bool Sequence::PopTask() {
-  AutoSchedulerLock auto_lock(lock_);
-  DCHECK(!queue_.empty());
+  // Delete the popped task outside the scope of |lock_|. This prevents a double
+  // acquisition of |lock_| if the task's destructor tries to post a task to
+  // this Sequence and reduces contention.
+  std::unique_ptr<Task> delete_outside_lock_scope;
+  bool sequence_empty_after_pop;

[robliao, 2016/07/21 17:53:18]

Variables have to be initialized at declaration, so go ahead and set it to
false.
https://google.github.io/styleguide/cppguide.html#Local_Variables

[fdoray, 2016/07/21 17:57:41]

Done.

 
-  const int priority_index =
-      static_cast<int>(queue_.front()->traits.priority());
-  DCHECK_GT(num_tasks_per_priority_[priority_index], 0U);
-  --num_tasks_per_priority_[priority_index];
+  {
+    AutoSchedulerLock auto_lock(lock_);
+    DCHECK(!queue_.empty());
+
+    const int priority_index =
+        static_cast<int>(queue_.front()->traits.priority());
+    DCHECK_GT(num_tasks_per_priority_[priority_index], 0U);
+    --num_tasks_per_priority_[priority_index];
+
+    delete_outside_lock_scope = std::move(queue_.front());
+    queue_.pop();
+    sequence_empty_after_pop = queue_.empty();
+  }
 
-  queue_.pop();
-  return queue_.empty();
+  return sequence_empty_after_pop;
 }
 
 SequenceSortKey Sequence::GetSortKey() const {