Don’t attempt to do periodic tasks in a secondary crashpad_handler

76a67a37b1d0 adds crashpad_handler’s --monitor-self argument, which
results in a second crashpad_handler instance running out of the same
database as the initial crashpad_handler instance that it monitors. The
two handlers start at nearly the same time, and will initially be on
precisely the same schedule for periodic tasks such as scanning for new
reports to upload and pruning the database. This is an unnecessary
duplication of effort.

This adds a new --no-periodic-tasks argument to crashpad_handler. When
the first instance of crashpad_handler starts a second to monitor it, it
will use this argument, which prevents the second instance from
performing these tasks.

When --no-periodic-tasks is in effect, crashpad_handler will still be
able to upload crash reports that it knows about by virtue of having
written them itself, but it will not scan the database for other pending
reports to upload.

Bug: crashpad:143
Test: crashpad_util_test ThreadSafeVector.ThreadSafeVector
Change-Id: I7b249dd7b6d5782448d8071855818f986b98ab5a
Reviewed-on: https://chromium-review.googlesource.com/473827
Reviewed-by: Robert Sesek <rsesek@chromium.org>

handler/crash_report_upload_thread.cc

@@ -17,6 +17,7 @@
 #include <errno.h>
 #include <time.h>
 
+#include <algorithm>
 #include <map>
 #include <memory>
 #include <vector>
@@ -29,7 +30,6 @@
 #include "snapshot/module_snapshot.h"
 #include "util/file/file_reader.h"
 #include "util/misc/metrics.h"
-#include "util/misc/uuid.h"
 #include "util/net/http_body.h"
 #include "util/net/http_multipart_builder.h"
 #include "util/net/http_transport.h"
@@ -139,15 +139,19 @@ class CallRecordUploadAttempt {
 
 CrashReportUploadThread::CrashReportUploadThread(CrashReportDatabase* database,
                                                  const std::string& url,
+                                                 bool watch_pending_reports,
                                                  bool rate_limit,
                                                  bool upload_gzip)
     : url_(url),
-      // Check for pending reports every 15 minutes, even in the absence of a
+      // When watching for pending reports, check every 15 minutes, even in the
-      // signal from the handler thread. This allows for failed uploads to be
+      // absence of a signal from the handler thread. This allows for failed
-      // retried periodically, and for pending reports written by other
+      // uploads to be retried periodically, and for pending reports written by
-      // processes to be recognized.
+      // other processes to be recognized.
-      thread_(15 * 60, this),
+      thread_(watch_pending_reports ? 15 * 60.0 : WorkerThread::kIndefiniteWait,
+              this),
+      known_pending_report_uuids_(),
       database_(database),
+      watch_pending_reports_(watch_pending_reports),
       rate_limit_(rate_limit),
       upload_gzip_(upload_gzip) {
 }
@@ -156,18 +160,43 @@ CrashReportUploadThread::~CrashReportUploadThread() {
 }
 
 void CrashReportUploadThread::Start() {
-  thread_.Start(0);
+  thread_.Start(watch_pending_reports_ ? 0.0 : WorkerThread::kIndefiniteWait);
 }
 
 void CrashReportUploadThread::Stop() {
   thread_.Stop();
 }
 
-void CrashReportUploadThread::ReportPending() {
+void CrashReportUploadThread::ReportPending(const UUID& report_uuid) {
+  known_pending_report_uuids_.PushBack(report_uuid);
   thread_.DoWorkNow();
 }
 
 void CrashReportUploadThread::ProcessPendingReports() {
+  std::vector<UUID> known_report_uuids = known_pending_report_uuids_.Drain();
+  for (const UUID& report_uuid : known_report_uuids) {
+    CrashReportDatabase::Report report;
+    if (database_->LookUpCrashReport(report_uuid, &report) !=
+        CrashReportDatabase::kNoError) {
+      continue;
+    }
+
+    ProcessPendingReport(report);
+
+    // Respect Stop() being called after at least one attempt to process a
+    // report.
+    if (!thread_.is_running()) {
+      return;
+    }
+  }
+
+  // Known pending reports are always processed (above). The rest of this
+  // function is concerned with scanning for pending reports not already known
+  // to this thread.
+  if (!watch_pending_reports_) {
+    return;
+  }
+
   std::vector<CrashReportDatabase::Report> reports;
   if (database_->GetPendingReports(&reports) != CrashReportDatabase::kNoError) {
     // The database is sick. It might be prudent to stop trying to poke it from
@@ -178,6 +207,15 @@ void CrashReportUploadThread::ProcessPendingReports() {
   }
 
   for (const CrashReportDatabase::Report& report : reports) {
+    if (std::find(known_report_uuids.begin(),
+                  known_report_uuids.end(),
+                  report.uuid) != known_report_uuids.end()) {
+      // An attempt to process the report already occurred above. The report is
+      // still pending, so upload must have failed. Don’t retry it immediately,
+      // it can wait until at least the next pass through this method.
+      continue;
+    }
+
     ProcessPendingReport(report);
 
     // Respect Stop() being called after at least one attempt to process a

handler/crash_report_upload_thread.h

@@ -20,6 +20,8 @@
 
 #include "base/macros.h"
 #include "client/crash_report_database.h"
+#include "util/misc/uuid.h"
+#include "util/stdlib/thread_safe_vector.h"
 #include "util/thread/worker_thread.h"
 
 namespace crashpad {
@@ -32,22 +34,28 @@ namespace crashpad {
 //! report has been added to the database by calling ReportPending().
 //!
 //! Independently of being triggered by ReportPending(), objects of this class
-//! periodically examine the database for pending reports. This allows failed
+//! can periodically examine the database for pending reports. This allows
-//! upload attempts for reports left in the pending state to be retried. It also
+//! failed upload attempts for reports left in the pending state to be retried.
-//! catches reports that are added without a ReportPending() signal being
+//! It also catches reports that are added without a ReportPending() signal
-//! caught. This may happen if crash reports are added to the database by other
+//! being caught. This may happen if crash reports are added to the database by
-//! processes.
+//! other processes.
 class CrashReportUploadThread : public WorkerThread::Delegate {
  public:
   //! \brief Constructs a new object.
   //!
   //! \param[in] database The database to upload crash reports from.
   //! \param[in] url The URL of the server to upload crash reports to.
+  //! \param[in] watch_pending_reports Whether to periodically check for new
+  //!     pending reports not already known to exist. When `false`, only an
+  //!     initial upload attempt will be made for reports known to exist by
+  //!     having been added by the ReportPending() method. No scans for new
+  //!     pending reports will be conducted.
   //! \param[in] rate_limit Whether uploads should be throttled to a (currently
   //!     hardcoded) rate.
   //! \param[in] upload_gzip Whether uploads should use `gzip` compression.
   CrashReportUploadThread(CrashReportDatabase* database,
                           const std::string& url,
+                          bool watch_pending_reports,
                           bool rate_limit,
                           bool upload_gzip);
   ~CrashReportUploadThread();
@@ -75,8 +83,11 @@ class CrashReportUploadThread : public WorkerThread::Delegate {
   //! \brief Informs the upload thread that a new pending report has been added
   //!     to the database.
   //!
+  //! \param[in] report_uuid The unique identifier of the newly added pending
+  //!     report.
+  //!
   //! This method may be called from any thread.
-  void ReportPending();
+  void ReportPending(const UUID& report_uuid);
 
  private:
   //! \brief The result code from UploadReport().
@@ -99,8 +110,13 @@ class CrashReportUploadThread : public WorkerThread::Delegate {
     kRetry,
   };
 
-  //! \brief Obtains all pending reports from the database, and calls
+  //! \brief Calls ProcessPendingReport() on pending reports.
-  //!     ProcessPendingReport() to process each one.
+  //!
+  //! Assuming Stop() has not been called, this will process reports that the
+  //! object has been made aware of in ReportPending(). Additionally, if the
+  //! object was constructed with \a watch_pending_reports, it will also scan
+  //! the crash report database for other pending reports, and process those as
+  //! well.
   void ProcessPendingReports();
 
   //! \brief Processes a single pending report from the database.
@@ -137,11 +153,13 @@ class CrashReportUploadThread : public WorkerThread::Delegate {
   //!     been called on any thread, as well as periodically on a timer.
   void DoWork(const WorkerThread* thread) override;
 
-  std::string url_;
+  const std::string url_;
   WorkerThread thread_;
+  ThreadSafeVector<UUID> known_pending_report_uuids_;
   CrashReportDatabase* database_;  // weak
-  bool rate_limit_;
+  const bool watch_pending_reports_;
-  bool upload_gzip_;
+  const bool rate_limit_;
+  const bool upload_gzip_;
 
   DISALLOW_COPY_AND_ASSIGN(CrashReportUploadThread);
 };

handler/crashpad_handler.md

@@ -151,7 +151,8 @@ establish the Crashpad client environment before running a program.
    become a client of the second one. The second instance will be started with
    the same **--annotation**, **--database**, **--monitor-self-annotation**,
    **--no-rate-limit**, **--no-upload-gzip**, and **--url** arguments as the
-   original one. The second instance will not be started with a
+   original one. The second instance will always be started with a
+   **--no-periodic-tasks** argument, and will not be started with a
    **--metrics-dir** argument even if the original instance was.
 
    Where supported by the underlying operating system, the second instance will
@@ -183,6 +184,20 @@ establish the Crashpad client environment before running a program.
    To prevent excessive accumulation of handler processes, _ARGUMENT_ must not
    be `--monitor-self`.
 
+ * **--no-periodic-tasks**
+
+   Do not scan for new pending crash reports or prune the crash report database.
+   Only crash reports recorded by this instance of the Crashpad handler will
+   become eligible for upload in this instance, and only a single initial upload
+   attempt will be made.
+
+   This option is not intended for general use. It is provided to prevent
+   multiple instances of the Crashpad handler from duplicating the effort of
+   performing the same periodic tasks. In normal use, the first instance of the
+   Crashpad handler will assume the responsibility for performing these tasks,
+   and will provide this argument to any second instance. See
+   **--monitor-self**.
+
  * **--no-rate-limit**
 
    Do not rate limit the upload of crash reports. By default uploads are

handler/handler_main.cc

@@ -109,6 +109,7 @@ void Usage(const base::FilePath& me) {
 "                              set a module annotation in the handler\n"
 "      --monitor-self-argument=ARGUMENT\n"
 "                              provide additional arguments to the second handler\n"
+"      --no-periodic-tasks     don't scan for new reports or prune the database\n"
 "      --no-rate-limit         don't rate limit crash uploads\n"
 "      --no-upload-gzip        don't use gzip compression when uploading\n"
 #if defined(OS_WIN)
@@ -142,6 +143,7 @@ struct Options {
   InitialClientData initial_client_data;
 #endif  // OS_MACOSX
   bool monitor_self;
+  bool periodic_tasks;
   bool rate_limit;
   bool upload_gzip;
 };
@@ -353,6 +355,7 @@ void MonitorSelf(const Options& options) {
     return;
   }
   std::vector<std::string> extra_arguments(options.monitor_self_arguments);
+  extra_arguments.push_back("--no-periodic-tasks");
   if (!options.rate_limit) {
     extra_arguments.push_back("--no-rate-limit");
   }
@@ -416,6 +419,7 @@ int HandlerMain(int argc,
     kOptionMonitorSelf,
     kOptionMonitorSelfAnnotation,
     kOptionMonitorSelfArgument,
+    kOptionNoPeriodicTasks,
     kOptionNoRateLimit,
     kOptionNoUploadGzip,
 #if defined(OS_WIN)
@@ -456,6 +460,7 @@ int HandlerMain(int argc,
      required_argument,
      nullptr,
      kOptionMonitorSelfArgument},
+    {"no-periodic-tasks", no_argument, nullptr, kOptionNoPeriodicTasks},
     {"no-rate-limit", no_argument, nullptr, kOptionNoRateLimit},
     {"no-upload-gzip", no_argument, nullptr, kOptionNoUploadGzip},
 #if defined(OS_WIN)
@@ -477,6 +482,7 @@ int HandlerMain(int argc,
 #if defined(OS_MACOSX)
   options.handshake_fd = -1;
 #endif
+  options.periodic_tasks = true;
   options.rate_limit = true;
   options.upload_gzip = true;
 
@@ -540,6 +546,10 @@ int HandlerMain(int argc,
         options.monitor_self_arguments.push_back(optarg);
         break;
       }
+      case kOptionNoPeriodicTasks: {
+        options.periodic_tasks = false;
+        break;
+      }
       case kOptionNoRateLimit: {
         options.rate_limit = false;
         break;
@@ -721,13 +731,19 @@ int HandlerMain(int argc,
   // TODO(scottmg): options.rate_limit should be removed when we have a
   // configurable database setting to control upload limiting.
   // See https://crashpad.chromium.org/bug/23.
-  CrashReportUploadThread upload_thread(
+  CrashReportUploadThread upload_thread(database.get(),
-      database.get(), options.url, options.rate_limit, options.upload_gzip);
+                                        options.url,
+                                        options.periodic_tasks,
+                                        options.rate_limit,
+                                        options.upload_gzip);
   upload_thread.Start();
 
-  PruneCrashReportThread prune_thread(database.get(),
+  std::unique_ptr<PruneCrashReportThread> prune_thread;
-                                      PruneCondition::GetDefault());
+  if (options.periodic_tasks) {
-  prune_thread.Start();
+    prune_thread.reset(new PruneCrashReportThread(
+        database.get(), PruneCondition::GetDefault()));
+    prune_thread->Start();
+  }
 
   CrashReportExceptionHandler exception_handler(database.get(),
                                                 &upload_thread,
@@ -744,7 +760,9 @@ int HandlerMain(int argc,
   exception_handler_server.Run(&exception_handler);
 
   upload_thread.Stop();
-  prune_thread.Stop();
+  if (prune_thread) {
+    prune_thread->Stop();
+  }
 
   return EXIT_SUCCESS;
 }

handler/mac/crash_report_exception_handler.cc

@@ -190,7 +190,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
       return KERN_FAILURE;
     }
 
-    upload_thread_->ReportPending();
+    upload_thread_->ReportPending(uuid);
   }
 
   if (client_options.system_crash_reporter_forwarding != TriState::kDisabled &&

handler/win/crash_report_exception_handler.cc

@@ -130,7 +130,7 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
       return termination_code;
     }
 
-    upload_thread_->ReportPending();
+    upload_thread_->ReportPending(uuid);
   }
 
   Metrics::ExceptionCaptureResult(Metrics::CaptureResult::kSuccess);

util/stdlib/thread_safe_vector.h

@@ -0,0 +1,63 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_UTIL_STDLIB_THREAD_SAFE_VECTOR_H_
+#define CRASHPAD_UTIL_STDLIB_THREAD_SAFE_VECTOR_H_
+
+#include <utility>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/synchronization/lock.h"
+
+namespace crashpad {
+
+//! \brief A wrapper for a `std::vector<>` that can be accessed safely from
+//!    multiple threads.
+//!
+//! This is not a drop-in replacement for `std::vector<>`. Only necessary
+//! operations are defined.
+template <typename T>
+class ThreadSafeVector {
+ public:
+  ThreadSafeVector() : vector_(), lock_() {}
+  ~ThreadSafeVector() {}
+
+  //! \brief Wraps `std::vector<>::%push_back()`.
+  void PushBack(const T& element) {
+    base::AutoLock lock_owner(lock_);
+    vector_.push_back(element);
+  }
+
+  //! \brief Atomically clears the underlying vector and returns its previous
+  //!     contents.
+  std::vector<T> Drain() {
+    std::vector<T> contents;
+    {
+      base::AutoLock lock_owner(lock_);
+      std::swap(vector_, contents);
+    }
+    return contents;
+  }
+
+ private:
+  std::vector<T> vector_;
+  base::Lock lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreadSafeVector);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_STDLIB_THREAD_SAFE_VECTOR_H_

util/stdlib/thread_safe_vector_test.cc

@@ -0,0 +1,91 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/stdlib/thread_safe_vector.h"
+
+#include "gtest/gtest.h"
+#include "util/thread/thread.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+constexpr int kElementsPerThread = 100;
+
+class ThreadSafeVectorTestThread : public Thread {
+ public:
+  ThreadSafeVectorTestThread() : thread_safe_vector_(nullptr), start_(0) {}
+  ~ThreadSafeVectorTestThread() {}
+
+  void SetTestParameters(ThreadSafeVector<int>* thread_safe_vector, int start) {
+    thread_safe_vector_ = thread_safe_vector;
+    start_ = start;
+  }
+
+  // Thread:
+  void ThreadMain() override {
+    for (int i = start_; i < start_ + kElementsPerThread; ++i) {
+      thread_safe_vector_->PushBack(i);
+    }
+  }
+
+ private:
+  ThreadSafeVector<int>* thread_safe_vector_;
+  int start_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreadSafeVectorTestThread);
+};
+
+TEST(ThreadSafeVector, ThreadSafeVector) {
+  ThreadSafeVector<int> thread_safe_vector;
+  std::vector<int> vector = thread_safe_vector.Drain();
+  EXPECT_TRUE(vector.empty());
+
+  ThreadSafeVectorTestThread threads[100];
+  for (size_t index = 0; index < arraysize(threads); ++index) {
+    threads[index].SetTestParameters(
+        &thread_safe_vector, static_cast<int>(index * kElementsPerThread));
+  }
+
+  for (size_t index = 0; index < arraysize(threads); ++index) {
+    threads[index].Start();
+
+    if (index % 10 == 0) {
+      // Drain the vector periodically to test that simultaneous Drain() and
+      // PushBack() operations work properly.
+      std::vector<int> drained = thread_safe_vector.Drain();
+      vector.insert(vector.end(), drained.begin(), drained.end());
+    }
+  }
+
+  for (ThreadSafeVectorTestThread& thread : threads) {
+    thread.Join();
+  }
+
+  std::vector<int> drained = thread_safe_vector.Drain();
+  vector.insert(vector.end(), drained.begin(), drained.end());
+  bool found[arraysize(threads) * kElementsPerThread] = {};
+  EXPECT_EQ(vector.size(), arraysize(found));
+  for (int element : vector) {
+    EXPECT_FALSE(found[element]) << element;
+    found[element] = true;
+  }
+
+  vector = thread_safe_vector.Drain();
+  EXPECT_TRUE(vector.empty());
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

util/synchronization/semaphore.h

@@ -15,6 +15,8 @@
 #ifndef CRASHPAD_UTIL_SYNCHRONIZATION_SEMAPHORE_H_
 #define CRASHPAD_UTIL_SYNCHRONIZATION_SEMAPHORE_H_
 
+#include <limits>
+
 #include "build/build_config.h"
 
 #if defined(OS_MACOSX)
@@ -30,6 +32,10 @@ namespace crashpad {
 //! \brief An anonymous in-process counting sempahore.
 class Semaphore {
  public:
+  //! \brief A TimedWait() argument that causes an indefinite wait.
+  static constexpr double kIndefiniteWait =
+      std::numeric_limits<double>::infinity();
+
   //! \brief Initializes the semaphore.
   //!
   //! \param[in] value The initial value of the semaphore.
@@ -51,7 +57,8 @@ class Semaphore {
   //! \brief Performs a timed wait (or “procure”) operation on the semaphore.
   //!
   //! \param[in] seconds The maximum number of seconds to wait for the operation
-  //!     to complete.
+  //!     to complete. If \a seconds is #kIndefiniteWait, this method behaves as
+  //!     Wait(), and will not time out.
   //!
   //! \return `false` if the wait timed out, `true` otherwise.
   //!

util/synchronization/semaphore_mac.cc

@@ -14,6 +14,8 @@
 
 #include "util/synchronization/semaphore.h"
 
+#include <cmath>
+
 #include "base/logging.h"
 
 namespace crashpad {
@@ -33,6 +35,12 @@ void Semaphore::Wait() {
 
 bool Semaphore::TimedWait(double seconds) {
   DCHECK_GE(seconds, 0.0);
+
+  if (std::isinf(seconds)) {
+    Wait();
+    return true;
+  }
+
   const dispatch_time_t timeout =
       dispatch_time(DISPATCH_TIME_NOW, seconds * NSEC_PER_SEC);
   return dispatch_semaphore_wait(semaphore_, timeout) == 0;

util/synchronization/semaphore_posix.cc

@@ -39,6 +39,12 @@ void Semaphore::Wait() {
 
 bool Semaphore::TimedWait(double seconds) {
   DCHECK_GE(seconds, 0.0);
+
+  if (std::isinf(seconds)) {
+    Wait();
+    return true;
+  }
+
   timespec timeout;
   timeout.tv_sec = seconds;
   timeout.tv_nsec = (seconds - trunc(seconds)) * 1E9;

util/synchronization/semaphore_test.cc

@@ -43,6 +43,18 @@ TEST(Semaphore, TimedWaitTimeout) {
   EXPECT_FALSE(semaphore.TimedWait(0.01));  // 10ms
 }
 
+TEST(Semaphore, TimedWaitInfinite_0) {
+  Semaphore semaphore(0);
+  semaphore.Signal();
+  EXPECT_TRUE(semaphore.TimedWait(std::numeric_limits<double>::infinity()));
+}
+
+TEST(Semaphore, TimedWaitInfinite_1) {
+  Semaphore semaphore(1);
+  EXPECT_TRUE(semaphore.TimedWait(std::numeric_limits<double>::infinity()));
+  semaphore.Signal();
+}
+
 struct ThreadMainInfo {
 #if defined(OS_POSIX)
   pthread_t pthread;

util/synchronization/semaphore_win.cc

@@ -14,6 +14,7 @@
 
 #include "util/synchronization/semaphore.h"
 
+#include <cmath>
 #include <limits>
 
 #include "base/logging.h"
@@ -38,6 +39,12 @@ void Semaphore::Wait() {
 
 bool Semaphore::TimedWait(double seconds) {
   DCHECK_GE(seconds, 0.0);
+
+  if (std::isinf(seconds)) {
+    Wait();
+    return true;
+  }
+
   DWORD rv = WaitForSingleObject(semaphore_, static_cast<DWORD>(seconds * 1E3));
   PCHECK(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT) << "WaitForSingleObject";
   return rv == WAIT_OBJECT_0;

util/thread/worker_thread.cc

@@ -15,7 +15,6 @@
 #include "util/thread/worker_thread.h"
 
 #include "base/logging.h"
-#include "util/synchronization/semaphore.h"
 #include "util/thread/thread.h"
 
 namespace crashpad {

util/thread/worker_thread.h

@@ -18,6 +18,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "util/synchronization/semaphore.h"
 
 namespace crashpad {
 
@@ -40,11 +41,16 @@ class WorkerThread {
     virtual ~Delegate() {}
   };
 
+  //! \brief A delay or interval argument that causes an indefinite wait.
+  static constexpr double kIndefiniteWait = Semaphore::kIndefiniteWait;
+
   //! \brief Creates a new WorkerThread that is not yet running.
   //!
   //! \param[in] work_interval The time interval in seconds at which the \a
   //!     delegate runs. The interval counts from the completion of
-  //!     Delegate::DoWork() to the next invocation.
+  //!     Delegate::DoWork() to the next invocation. This can be
+  //!     #kIndefiniteWait if work should only be done when DoWorkNow() is
+  //!     called.
   //! \param[in] delegate The work delegate to invoke every interval.
   WorkerThread(double work_interval, Delegate* delegate);
   ~WorkerThread();
@@ -55,7 +61,8 @@ class WorkerThread {
   //!
   //! \param[in] initial_work_delay The amount of time in seconds to wait
   //!     before invoking the \a delegate for the first time. Pass `0` for
-  //!     no delay.
+  //!     no delay. This can be #kIndefiniteWait if work should not be done
+  //!     until DoWorkNow() is called.
   void Start(double initial_work_delay);
 
   //! \brief Stops the worker thread from running.

util/util.gyp

@@ -168,6 +168,7 @@
         'stdlib/strlcpy.h',
         'stdlib/strnlen.cc',
         'stdlib/strnlen.h',
+        'stdlib/thread_safe_vector.h',
         'string/split_string.cc',
         'string/split_string.h',
         'synchronization/semaphore_mac.cc',

util/util_test.gyp

@@ -87,6 +87,7 @@
         'stdlib/string_number_conversion_test.cc',
         'stdlib/strlcpy_test.cc',
         'stdlib/strnlen_test.cc',
+        'stdlib/thread_safe_vector_test.cc',
         'string/split_string_test.cc',
         'synchronization/semaphore_test.cc',
         'thread/thread_log_messages_test.cc',