Fix memory leak in 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 <vector>
@@ skipping 27 matching lines @@
   tracked_objects::Location location;
   Closure task;
 };
 
 }  // namespace
 
 // Worker ---------------------------------------------------------------------
 
 class SequencedWorkerPool::Worker : public SimpleThread {
  public:
-  // Hold a ref to |worker_pool|, since we want to keep it around even
-  // if it doesn't join our thread.  Note that this (deliberately)
+  // Hold a (cyclic) ref to |worker_pool|, since we want to keep it
+  // around as long as we are running.  Note that this (deliberately)

[brettw, 2012/02/28 04:38:46]

Is the leaks on shutdown comment still valid? If so, it needs more explanation
since it doesn't make sense to me.

[akalin, 2012/02/28 22:10:33]

Done.

   // leaks on shutdown.
   Worker(const scoped_refptr<SequencedWorkerPool>& worker_pool,
          int thread_number,
          const std::string& thread_name_prefix);
   virtual ~Worker();
 
   // SimpleThread implementation. This actually runs the background thread.
   virtual void Run() OVERRIDE;
 
  private:
-  const scoped_refptr<SequencedWorkerPool> worker_pool_;
+  scoped_refptr<SequencedWorkerPool> worker_pool_;
 
   DISALLOW_COPY_AND_ASSIGN(Worker);
 };
 
 // Inner ----------------------------------------------------------------------
 
 class SequencedWorkerPool::Inner {
  public:
   // Take a raw pointer to |worker| to avoid cycles (since we're owned
   // by it).
@@ skipping 148 matching lines @@
 SequencedWorkerPool::Worker::~Worker() {
 }
 
 void SequencedWorkerPool::Worker::Run() {
   // Just jump back to the Inner object to run the thread, since it has all the
   // tracking information and queues. It might be more natural to implement
   // using DelegateSimpleThread and have Inner implement the Delegate to avoid
   // having these worker objects at all, but that method lacks the ability to
   // send thread-specific information easily to the thread loop.
   worker_pool_->inner_->ThreadLoop(this);
+  // Release our cyclic reference once we're done.
+  worker_pool_ = NULL;
 }
 
 // Inner definitions ---------------------------------------------------------
 
 SequencedWorkerPool::Inner::Inner(
     SequencedWorkerPool* worker_pool,
     size_t max_threads,
     const std::string& thread_name_prefix)
     : worker_pool_(worker_pool),
       last_sequence_number_(0),
@@ skipping 456 matching lines @@
 
 void SequencedWorkerPool::Shutdown() {
   inner_->Shutdown();
 }
 
 void SequencedWorkerPool::SetTestingObserver(TestingObserver* observer) {
   inner_->SetTestingObserver(observer);
 }
 
 }  // namespace base