Base: Fix another deadlock during shutdown of SequencedWorkerPool.

base/threading/sequenced_worker_pool.cc

 // Copyright (c) 2012 The Chromium 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 "base/threading/sequenced_worker_pool.h"
 
 #include <list>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 785 matching lines @@
         // but only worker threads are allowed to post tasks at that time, and
         // the workers responsible for posting those tasks will be available
         // to run them. Also, there may be some tasks stuck behind running
         // ones with the same sequence token, but additional threads won't
         // help this case.
         if (shutdown_called_ && blocking_shutdown_pending_task_count_ == 0) {
           AutoUnlock unlock(lock_);
           delete_these_outside_lock.clear();
           break;
         }
+
+        // No work was found, but there are tasks that need deletion. The
+        // deletion must happen outside of the lock. Since the lock has been

[brettw, 2015/03/12 23:12:42]

I don't follow this, maybe the comment needs to be clarified.

This talks about the lock "has been released" but when this code is called, the
lock is still held.

[erikchen, 2015/03/12 23:24:52]

I broke the comment in two, moved the location of the second comment, and
rewrote it to be more precise. I've reproduced it here for your convenience:

 // Since the lock has been released, |status| may no longer be
 // accurate. It might read GET_WORK_WAIT even if there are tasks
 // ready to perform work. Jump to the top of the loop to recalculate
 // |status|.

+        // released, the current state about whether there is more work is no
+        // longer valid, and needs to be recalculated by jumping to the top of
+        // the loop.
+        if (delete_these_outside_lock.size()) {
+          AutoUnlock unlock(lock_);
+          delete_these_outside_lock.clear();
+          continue;
+        }
+
         waiting_thread_count_++;
 
         switch (status) {
           case GET_WORK_NOT_FOUND:
             has_work_cv_.Wait();
             break;
           case GET_WORK_WAIT:
             has_work_cv_.TimedWait(wait_time);
             break;
           default:
@@ skipping 459 matching lines @@
 void SequencedWorkerPool::Shutdown(int max_new_blocking_tasks_after_shutdown) {
   DCHECK(constructor_message_loop_->BelongsToCurrentThread());
   inner_->Shutdown(max_new_blocking_tasks_after_shutdown);
 }
 
 bool SequencedWorkerPool::IsShutdownInProgress() {
   return inner_->IsShutdownInProgress();
 }
 
 }  // namespace base