Improve usage documentation of scoped task environments.

base/task_scheduler/scheduler_single_thread_task_runner_manager.cc

 // Copyright 2017 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/task_scheduler/scheduler_single_thread_task_runner_manager.h"
 
 #include <algorithm>
 #include <memory>
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/debug/stack_trace.h"
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/atomic_flag.h"
 #include "base/task_scheduler/delayed_task_manager.h"
 #include "base/task_scheduler/scheduler_worker.h"
 #include "base/task_scheduler/sequence.h"
 #include "base/task_scheduler/task.h"
 #include "base/task_scheduler/task_tracker.h"
 #include "base/task_scheduler/task_traits.h"
@@ skipping 253 matching lines @@
     // Tasks are never nested within the task scheduler.
     return PostDelayedTask(from_here, std::move(closure), delay);
   }
 
   bool RunsTasksOnCurrentThread() const override {
     return GetDelegate()->RunsTasksOnCurrentThread();
   }
 
  private:
   ~SchedulerSingleThreadTaskRunner() override {
+    // Note: this will crash if SchedulerSingleThreadTaskRunnerManager() is

[robliao, 2017/05/04 21:54:27]

Nit: Capitalize 'this'
Nit: No () as we're talking about the object itself.

[gab, 2017/05/04 22:27:41]

Done.

+    // incorrectly destroyed first in tests (in production the TaskScheduler and
+    // all of its state are intentionally leaked after
+    // TaskScheduler::Shutdown(). See ~SchedulerSingleThreadTaskRunnerManager()
+    // for more details.
     outer_->UnregisterSchedulerWorker(worker_);
   }
 
   void PostTaskNow(std::unique_ptr<Task> task) {
     scoped_refptr<Sequence> sequence = GetDelegate()->sequence();
     // If |sequence| is null, then the thread is effectively gone (either
     // shutdown or joined).
     if (!sequence)
       return;
 
@@ skipping 21 matching lines @@
     : task_tracker_(task_tracker), delayed_task_manager_(delayed_task_manager) {
   DCHECK(task_tracker_);
   DCHECK(delayed_task_manager_);
 }
 
 SchedulerSingleThreadTaskRunnerManager::
     ~SchedulerSingleThreadTaskRunnerManager() {
 #if DCHECK_IS_ON()
   size_t workers_unregistered_during_join =
       subtle::NoBarrier_Load(&workers_unregistered_during_join_);
-  DCHECK_EQ(workers_unregistered_during_join, workers_.size())
-      << "There cannot be outstanding SingleThreadTaskRunners upon destruction "
-         "of SchedulerSingleThreadTaskRunnerManager or the Task Scheduler";
+  // Log an ERROR instead of DCHECK'ing as it's often useful to have both the
+  // stack trace of this call and the crash stack trace of the upcoming
+  // out-of-order ~SchedulerSingleThreadTaskRunner() call to know what to flip.
+  DLOG_IF(ERROR, workers_unregistered_during_join != workers_.size())
+      << "Expect incoming crash in ~SchedulerSingleThreadTaskRunner()!!! There "
+         "cannot be outstanding SingleThreadTaskRunners upon destruction "
+         "of SchedulerSingleThreadTaskRunnerManager in tests "
+      << workers_.size() - workers_unregistered_during_join << " outstanding). "
+      << "Hint: If you're hitting this it's most likely because your test "

[robliao, 2017/05/04 21:54:28]

s/Hint/Hint 1/

[gab, 2017/05/04 22:27:41]

Done.

+         "fixture is destroying its TaskScheduler too early (e.g. via "
+         "~base::test::ScopedTaskEnvironment() or "

[robliao, 2017/05/04 21:54:27]

Nit: base::test::~ScopedTaskEnvironment() and
content::~TestBrowserThreadBundle()

[gab, 2017/05/04 22:27:41]

Done.

+         "~content::TestBrowserThreadBundle()). Refer to the following stack "
+         "trace to know what caused this destruction as well as to the "
+         "upcoming crash in ~SchedulerSingleThreadTaskRunner() to know what "
+         "should have happened before. "
+         "Hint hint: base::test::ScopedTaskEnvironment et al. should typically "

[robliao, 2017/05/04 21:54:27]

s/Hint hint/Hint 2/

[gab, 2017/05/04 22:27:41]

Done.

+         "be the first member in a test fixture to ensure it's initialized "
+         "first and destroyed last.\n"
+      << base::debug::StackTrace().ToString();
 #endif
 }
 
 void SchedulerSingleThreadTaskRunnerManager::Start() {
   decltype(workers_) workers_to_start;
   {
     AutoSchedulerLock auto_lock(lock_);
     started_ = true;
     workers_to_start = workers_;
   }
@@ skipping 121 matching lines @@
                      });
     DCHECK(worker_iter != workers_.end());
     worker_to_destroy = std::move(*worker_iter);
     workers_.erase(worker_iter);
   }
   worker_to_destroy->Cleanup();
 }
 
 }  // namespace internal
 }  // namespace base