Ensure that SequencedWorkerPools in tests don't outlive their tests

base/threading/sequenced_worker_pool_unittest.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 <algorithm>
 
 #include "base/bind.h"
+#include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/synchronization/condition_variable.h"
 #include "base/synchronization/lock.h"
 #include "base/task_runner_test_template.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/tracked_objects.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 
@@ skipping 115 matching lines @@
 
   base::ConditionVariable cond_var_;
 
   // Protected by lock_.
   std::vector<int> complete_sequence_;
 
   // Counter of the number of "block" workers that have started.
   size_t started_events_;
 };
 
-class SequencedWorkerPoolTest : public testing::Test,
-                                public SequencedWorkerPool::TestingObserver {
+// Wrapper around SequencedWorkerPool that blocks destruction until
+// the pool is actually destroyed.  This is so that a
+// SequencedWorkerPool from one test doesn't outlive its test and
+// cause strange races with other tests that touch global stuff (like
+// histograms and logging).  However, this requires that nothing else
+// on this thread holds a ref to the pool when the
+// SequencedWorkerPoolOwner is destroyed.
+class SequencedWorkerPoolOwner : public SequencedWorkerPool::TestingObserver {
+ public:
+  SequencedWorkerPoolOwner(size_t max_threads,
+                           const std::string& thread_name_prefix)
+      : constructor_message_loop_(MessageLoop::current()),
+        pool_(new SequencedWorkerPool(max_threads, thread_name_prefix)) {
+    pool_->SetTestingObserver(this);
+  }
+
+  virtual ~SequencedWorkerPoolOwner() {
+    pool_ = NULL;
+    MessageLoop::current()->Run();
+  }
+
+  // Don't change the return pool's testing observer.
+  const scoped_refptr<SequencedWorkerPool>& pool() {
+    return pool_;
+  }
+
+  // The given callback will be called on WillWaitForShutdown().
+  void SetWillWaitForShutdownCallback(const Closure& callback) {
+    will_wait_for_shutdown_callback_ = callback;
+  }
+
+ private:
+  // SequencedWorkerPool::TestingObserver implementation.
+  virtual void WillWaitForShutdown() OVERRIDE {
+    if (!will_wait_for_shutdown_callback_.is_null()) {
+      will_wait_for_shutdown_callback_.Run();
+    }
+  }
+
+  virtual void OnDestruct() OVERRIDE {
+    constructor_message_loop_->PostTask(
+        FROM_HERE,
+        constructor_message_loop_->QuitClosure());
+  }
+
+  MessageLoop* const constructor_message_loop_;
+  scoped_refptr<SequencedWorkerPool> pool_;
+  Closure will_wait_for_shutdown_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(SequencedWorkerPoolOwner);
+};
+
+class SequencedWorkerPoolTest : public testing::Test {
  public:
   SequencedWorkerPoolTest()
-      : pool_(new SequencedWorkerPool(kNumWorkerThreads, "test")),
-        tracker_(new TestTracker) {
-    pool_->SetTestingObserver(this);
-  }
-  ~SequencedWorkerPoolTest() {
+      : pool_owner_(kNumWorkerThreads, "test"),
+        tracker_(new TestTracker) {}
+
+  ~SequencedWorkerPoolTest() {}
+
+  virtual void SetUp() {}
+
+  virtual void TearDown() {
+    pool()->Shutdown();
   }
 
-  virtual void SetUp() {
+  const scoped_refptr<SequencedWorkerPool>& pool() {
+    return pool_owner_.pool();
   }
-  virtual void TearDown() {
-    pool_->Shutdown();
+  TestTracker* tracker() { return tracker_.get(); }
+
+  void SetWillWaitForShutdownCallback(const Closure& callback) {
+    pool_owner_.SetWillWaitForShutdownCallback(callback);
   }
 
-  const scoped_refptr<SequencedWorkerPool>& pool() { return pool_; }
-  TestTracker* tracker() { return tracker_.get(); }
-
   // Ensures that the given number of worker threads is created by adding
   // tasks and waiting until they complete. Worker thread creation is
   // serialized, can happen on background threads asynchronously, and doesn't
   // happen any more at shutdown. This means that if a test posts a bunch of
   // tasks and calls shutdown, fewer workers will be created than the test may
   // expect.
   //
   // This function ensures that this condition can't happen so tests can make
   // assumptions about the number of workers active. See the comment in
   // PrepareToStartAdditionalThreadIfNecessary in the .cc file for more
@@ skipping 13 matching lines @@
     tracker()->WaitUntilTasksBlocked(kNumWorkerThreads);
 
     // Now wake them up and wait until they're done.
     blocker.Unblock(kNumWorkerThreads);
     tracker()->WaitUntilTasksComplete(kNumWorkerThreads);
 
     // Clean up the task IDs we added.
     tracker()->ClearCompleteSequence();
   }
 
- protected:
-  // This closure will be executed right before the pool blocks on shutdown.
-  base::Closure before_wait_for_shutdown_;
-
  private:
-  // SequencedWorkerPool::TestingObserver implementation.
-  virtual void WillWaitForShutdown() {
-    if (!before_wait_for_shutdown_.is_null())
-      before_wait_for_shutdown_.Run();
-  }
-
-  const scoped_refptr<SequencedWorkerPool> pool_;
+  MessageLoop message_loop_;
+  SequencedWorkerPoolOwner pool_owner_;
   const scoped_refptr<TestTracker> tracker_;
 };
 
 // Checks that the given number of entries are in the tasks to complete of
 // the given tracker, and then signals the given event the given number of
 // times. This is used to wakt up blocked background threads before blocking
 // on shutdown.
 void EnsureTasksToCompleteCountAndUnblock(scoped_refptr<TestTracker> tracker,
                                           size_t expected_tasks_to_complete,
                                           ThreadBlocker* blocker,
@@ skipping 46 matching lines @@
 
   std::vector<int> result = tracker()->WaitUntilTasksComplete(kNumTasks);
   EXPECT_EQ(kNumTasks, result.size());
 }
 
 // Tests that posting a bunch of tasks (many more than the number of
 // worker threads) to two pools simultaneously runs them all twice.
 // This test is meant to shake out any concurrency issues between
 // pools (like histograms).
 TEST_F(SequencedWorkerPoolTest, LotsOfTasksTwoPools) {
-  scoped_refptr<SequencedWorkerPool> pool1(
-      new SequencedWorkerPool(kNumWorkerThreads, "test1"));
-  scoped_refptr<SequencedWorkerPool> pool2(
-      new SequencedWorkerPool(kNumWorkerThreads, "test2"));
+  SequencedWorkerPoolOwner pool1(kNumWorkerThreads, "test1");
+  SequencedWorkerPoolOwner pool2(kNumWorkerThreads, "test2");
 
   base::Closure slow_task = base::Bind(&TestTracker::SlowTask, tracker(), 0);
-  pool1->PostWorkerTask(FROM_HERE, slow_task);
-  pool2->PostWorkerTask(FROM_HERE, slow_task);
+  pool1.pool()->PostWorkerTask(FROM_HERE, slow_task);
+  pool2.pool()->PostWorkerTask(FROM_HERE, slow_task);
 
   const size_t kNumTasks = 20;
   for (size_t i = 1; i < kNumTasks; i++) {
     base::Closure fast_task =
         base::Bind(&TestTracker::FastTask, tracker(), i);
-    pool1->PostWorkerTask(FROM_HERE, fast_task);
-    pool2->PostWorkerTask(FROM_HERE, fast_task);
+    pool1.pool()->PostWorkerTask(FROM_HERE, fast_task);
+    pool2.pool()->PostWorkerTask(FROM_HERE, fast_task);
   }
 
   std::vector<int> result =
       tracker()->WaitUntilTasksComplete(2*kNumTasks);
   EXPECT_EQ(2*kNumTasks, result.size());
+
+  pool2.pool()->Shutdown();
+  pool1.pool()->Shutdown();
 }
 
 // Test that tasks with the same sequence token are executed in order but don't
 // affect other tasks.
 TEST_F(SequencedWorkerPoolTest, Sequence) {
   // Fill all the worker threads except one.
   const size_t kNumBackgroundTasks = kNumWorkerThreads - 1;
   ThreadBlocker background_blocker;
   for (size_t i = 0; i < kNumBackgroundTasks; i++) {
     pool()->PostWorkerTask(FROM_HERE,
@@ skipping 71 matching lines @@
   pool()->PostWorkerTaskWithShutdownBehavior(
       FROM_HERE,
       base::Bind(&TestTracker::FastTask, tracker(), 101),
       SequencedWorkerPool::SKIP_ON_SHUTDOWN);
   pool()->PostWorkerTaskWithShutdownBehavior(
       FROM_HERE,
       base::Bind(&TestTracker::FastTask, tracker(), 102),
       SequencedWorkerPool::BLOCK_SHUTDOWN);
 
   // Shutdown the worker pool. This should discard all non-blocking tasks.
-  before_wait_for_shutdown_ =
+  SetWillWaitForShutdownCallback(
       base::Bind(&EnsureTasksToCompleteCountAndUnblock,
                  scoped_refptr<TestTracker>(tracker()), 0,
-                 &blocker, kNumWorkerThreads);
+                 &blocker, kNumWorkerThreads));
   pool()->Shutdown();
 
   std::vector<int> result = tracker()->WaitUntilTasksComplete(4);
 
   // The kNumWorkerThread items should have completed, plus the BLOCK_SHUTDOWN
   // one, in no particular order.
   ASSERT_EQ(4u, result.size());
   for (size_t i = 0; i < kNumWorkerThreads; i++) {
     EXPECT_TRUE(std::find(result.begin(), result.end(), static_cast<int>(i)) !=
                 result.end());
@@ skipping 29 matching lines @@
   EXPECT_EQ(1u, result.size());
 }
 
 class SequencedWorkerPoolTaskRunnerTestDelegate {
  public:
   SequencedWorkerPoolTaskRunnerTestDelegate() {}
 
   ~SequencedWorkerPoolTaskRunnerTestDelegate() {}
 
   void StartTaskRunner() {
-    worker_pool_ =
-        new SequencedWorkerPool(10, "SequencedWorkerPoolTaskRunnerTest");
+    pool_owner_.reset(
+        new SequencedWorkerPoolOwner(10, "SequencedWorkerPoolTaskRunnerTest"));
   }
 
   scoped_refptr<SequencedWorkerPool> GetTaskRunner() {
-    return worker_pool_;
+    return pool_owner_->pool();
   }
 
   void StopTaskRunner() {
-    worker_pool_->Shutdown();
-    worker_pool_ = NULL;
+    pool_owner_->pool()->Shutdown();
+    // Don't reset |pool_owner_| here, as the test may still hold a
+    // reference to the pool.
   }
 
   bool TaskRunnerHandlesNonZeroDelays() const {
     // TODO(akalin): Set this to true once SequencedWorkerPool handles
     // non-zero delays.
     return false;
   }
 
  private:
-  scoped_refptr<SequencedWorkerPool> worker_pool_;
+  MessageLoop message_loop_;
+  scoped_ptr<SequencedWorkerPoolOwner> pool_owner_;
 };
 
 INSTANTIATE_TYPED_TEST_CASE_P(
     SequencedWorkerPool, TaskRunnerTest,
     SequencedWorkerPoolTaskRunnerTestDelegate);
 
 }  // namespace
 
 }  // namespace base