Add a sequenced worker pool

content/common/sequenced_worker_pool.h

+// Copyright (c) 2011 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.
+
+#ifndef CONTENT_COMMON_SEQUENCED_WORKER_POOL_H_
+#define CONTENT_COMMON_SEQUENCED_WORKER_POOL_H_
+#pragma once
+
+#include <string>
+
+#include "base/callback.h"
+#include "base/memory/linked_ptr.h"
+#include "base/memory/ref_counted.h"
+#include "base/tracked_objects.h"
+#include "content/common/content_export.h"
+
+// A worker thread pool that enforces ordering between sets of tasks. It also
+// allows you to specify what should happen to your tasks on shutdown.
+//
+// To enforce ordering, get a unique sequence token from the pool and post all
+// tasks you want to order with the token. All tasks with the same token are
+// guaranteed to execute serially, though not necessarily on the same thread.
+//
+// Example:
+//   SequencedWorkerPool::SequenceToken token = pool.GetSequenceToken();
+//   pool.PostSequencedWorkerTask(token, SequencedWorkerPool::SKIP_ON_SHUTDOWN,
+//                                FROM_HERE, base::Bind(...));
+//   pool.PostSequencedWorkerTask(token, SequencedWorkerPool::SKIP_ON_SHUTDOWN,
+//                                FROM_HERE, base::Bind(...));
+//
+// You can make named sequence tokens to make it easier to share a token
+// across different components.
+//
+// You can also post tasks to the pool without ordering using PostWorkerTask.
+// These will be executed in an unspecified order. The order of execution
+// between tasks with different sequence tokens is also unspecified.
+//
+// This class is designed to be leaked on shutdown to allow the
+// CONTINUE_ON_SHUTDOWN behavior to be implemented. To enforce the
+// BLOCK_SHUTDOWN behavior, you must call Shutdown() which will wait until
+// the necessary tasks have completed.
+//
+// Implementation note: This does not use a base::WorkerPool since that does
+// not enforce shutdown semantics or allow us to specify how many worker
+// threads to run. For the typical use case of random background work, we don't
+// necessarily want to be super aggressive about creating threads.
+class CONTENT_EXPORT SequencedWorkerPool {
+ public:
+  // Defines what should happen to a task posted to the worker pool on shutdown.
+  enum WorkerShutdown {
+    // Tasks posted with this mode which have not run at shutdown will be
+    // deleted rather than run, and any tasks with this mode running at

[jar (doing other things), 2011/12/09 19:01:48]

Your comments came later about how deletion is not guaranteed... but you should
update this to at least say "deleted (if time permits) rather than run.."

+    // shutdown will be ignored (the worker thread will not be joined).
+    //
+    // This option provides a nice way to post stuff you don't want blocking
+    // shutdown. For example, you might be doing a slow DNS lookup and if it's
+    // blocked on the OS, you may not want to stop shutdown, since the result
+    // doesn't really matter at that point.
+    //
+    // However, you need to be very careful what you do in your callback when
+    // you use this option. Since the thread will continue to run until the OS
+    // terminates the process, the app can be in the process of tearing down
+    // when you're running. This means any singletons or global objects you
+    // use may suddenly become invalid out from under you. For this reason,
+    // it's best to use this only for slow but simple operations like the DNS
+    // example.
+    CONTINUE_ON_SHUTDOWN,
+
+    // Tasks posted with this mode that have not started executing at shutdown
+    // will be deleted rather than executed. However, tasks already in progress

[jar (doing other things), 2011/12/09 19:01:48]

again: "...deleted (if time permits) and will not be executed."

+    // will be completed.
+    SKIP_ON_SHUTDOWN,
+
+    // Tasks posted with this mode will block browser shutdown until they're
+    // executed. Since this can have significant performance implications, use
+    // sparingly.
+    //
+    // Generally, this should be used only for user data, for example, a task
+    // writing a preference file.
+    //
+    // If a task is posted during shutdown, it will not get run since the
+    // workers may already be stopped. In this case, the post operation will
+    // fail (return false) and the task will be deleted.
+    BLOCK_SHUTDOWN,
+  };
+
+  // Opaque identifier that defines sequencing of tasks posted to the worker
+  // pool. See NewSequenceToken().
+  class SequenceToken {
+   public:
+    SequenceToken() : id_(0) {}
+    ~SequenceToken() {}
+
+    bool Equals(const SequenceToken& other) const {
+      return id_ == other.id_;
+    }
+
+   private:
+    friend SequencedWorkerPool;
+
+    SequenceToken(int id) : id_(id) {}

[jar (doing other things), 2011/12/09 19:01:48]

nit: add explicit

+
+    int id_;
+  };
+
+  // Allows tests to perform certain actions.
+  class TestingObserver {
+   public:
+    virtual void WillWaitForShutdown() = 0;

[jar (doing other things), 2011/12/09 19:01:48]

nit: add virtual destructor.

+  };
+
+  SequencedWorkerPool(size_t max_threads);

[jar (doing other things), 2011/12/09 19:01:48]

nit: explicit

+  ~SequencedWorkerPool();
+
+  // Returns a unique token that can be used to sequence tasks posted to
+  // PostSequencedWorkerTask(). Valid tokens are alwys nonzero.
+  SequenceToken GetSequenceToken();
+
+  // Returns the sequence token associated with the given name. Calling this
+  // function multiple times with the same string will always produce the
+  // same sequence token. If the name has not been used before, a new token
+  // will be created.
+  SequenceToken GetNamedSequenceToken(const std::string& name);
+
+  // Posts the given task for execution in the worker pool. Tasks posted with
+  // this function will execute in an unspecified order on a background thread.
+  // Returns true if the task was posted. If your tasks have ordering
+  // requirements, see PostSequencedWorkerTask().
+  //
+  // This class will attempt to delete tasks that aren't run
+  // (non-block-shutdown semantics) but can't guarantee that this happens. If
+  // all worker threads are busy running CONTINUE_ON_SHUTDOWN tasks, there
+  // will be no workers available to delete these tasks. And there may be
+  // tasks with the same sequence token behind those CONTINUE_ON_SHUTDOWN
+  // tasks. Deleting those tasks before the previous one has completed could
+  // cause nondeterministic crashes because the task could be keeping some
+  // objects alive which do work in their destructor, which could voilate the
+  // assumptions of the running task.
+  //
+  // Returns true if the task was posted successfully. This may fail during
+  // shutdown regardless of the specified ShutdownBehavior.
+  bool PostWorkerTask(WorkerShutdown shutdown_behavior,
+                      const tracked_objects::Location& from_here,
+                      const base::Closure& task);
+
+  // Like PostWorkerTask above, but provides sequencing semantics. This means
+  // that tasks posted with the same sequence token (see GetSequenceToken())
+  // are guaranteed to execute in order. This is useful in cases where you're
+  // doing operations that may depend on previous ones, like appending to a
+  // file.
+  //
+  // Returns true if the task was posted successfully. This may fail during
+  // shutdown regardless of the specified ShutdownBehavior.
+  bool PostSequencedWorkerTask(SequenceToken sequence_token,

[jar (doing other things), 2011/12/09 19:01:48]

I'm tempted to believe that tasks should by default be required for shutdown.  I
think it takes a lot of thinking to be sure you're an "optional" task.  As a
result, I'd like to not bother this arg list with the shutdown behavior, and
have  a second method like:
PostNonCrucialSequencedWorkerTask(...)

WDYT?  Do you really want every posted task to think through this and make a
choice?  I'm also wary that folks will "copy" the calling pattern, and not think
through the ramifications.

Today, all FileThread tasks are "important," as I *think* we  join the thread on
completion (yes??).  As a result, the default migration should be to those
semantics.

+                               WorkerShutdown shutdown_behavior,
+                               const tracked_objects::Location& from_here,
+                               const base::Closure& task);
+
+  // Implements the worker pool shutdown. This should be called during app
+  // shutdown, and will discard/join with appropriate tasks before returning.
+  // After this call, subsequent calls to post tasks will fail.
+  void Shutdown();
+
+  // Called by tests to set the testing observer. This is NULL by default
+  // and ownership of the pointer is kept with the caller.
+  void SetTestingObserver(TestingObserver* observer);
+
+ private:
+  class Inner;
+  class Worker;
+
+  friend class Inner;
+  friend class Worker;
+
+  scoped_refptr<Inner> inner_;
+
+  DISALLOW_COPY_AND_ASSIGN(SequencedWorkerPool);
+};
+
+#endif  // CONTENT_COMMON_SEQUENCED_WORKER_POOL_H_