[ThreadPool]: Fix flaky JoinForTesting

Issue: Idle thread creation is not disallowed while JoinForTesting().
Fix: prevent thread creation in EnsureEnoughWorkersLockRequired with join_for_testing_started_.

Bug: 1009536
Change-Id: I3f6f18efa5e950c69fef1f7d4458d703ca1d6056
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834201
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703347}

base/task/thread_pool/thread_group_impl.cc

@@ -482,10 +482,6 @@ void ThreadGroupImpl::WaitForWorkersCleanedUpForTesting(size_t n) {
 }
 
 void ThreadGroupImpl::JoinForTesting() {
-#if DCHECK_IS_ON()
-  join_for_testing_started_.Set();
-#endif
-
   decltype(workers_) workers_copy;
   {
     CheckedAutoLock auto_lock(lock_);
@@ -494,6 +490,8 @@ void ThreadGroupImpl::JoinForTesting() {
     DCHECK_GT(workers_.size(), size_t(0))
         << "Joined an unstarted thread group.";
 
+    join_for_testing_started_ = true;
+
     // Ensure WorkerThreads in |workers_| do not attempt to cleanup while
     // being joined.
     worker_cleanup_disallowed_for_testing_ = true;
@@ -711,6 +709,7 @@ bool ThreadGroupImpl::WorkerThreadDelegateImpl::CanCleanupLockRequired(
 
 void ThreadGroupImpl::WorkerThreadDelegateImpl::CleanupLockRequired(
     WorkerThread* worker) {
+  DCHECK(!outer_->join_for_testing_started_);
   DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
 
   if (outer_->num_tasks_before_detach_histogram_) {
@@ -759,7 +758,7 @@ void ThreadGroupImpl::WorkerThreadDelegateImpl::OnMainExit(
     // |workers_| by the time the thread is about to exit. (except in the cases
     // where the thread group is no longer going to be used - in which case,
     // it's fine for there to be invalid workers in the thread group.
-    if (!shutdown_complete && !outer_->join_for_testing_started_.IsSet()) {
+    if (!shutdown_complete && !outer_->join_for_testing_started_) {
       DCHECK(!outer_->idle_workers_stack_.Contains(worker));
       DCHECK(!ContainsWorker(outer_->workers_, worker));
     }
@@ -947,6 +946,7 @@ void ThreadGroupImpl::MaintainAtLeastOneIdleWorkerLockRequired(
 scoped_refptr<WorkerThread>
 ThreadGroupImpl::CreateAndRegisterWorkerLockRequired(
     ScopedWorkersExecutor* executor) {
+  DCHECK(!join_for_testing_started_);
   DCHECK_LT(workers_.size(), max_tasks_);
   DCHECK_LT(workers_.size(), kMaxNumberOfWorkers);
   DCHECK(idle_workers_stack_.IsEmpty());
@@ -1016,7 +1016,7 @@ void ThreadGroupImpl::DidUpdateCanRunPolicy() {
 void ThreadGroupImpl::EnsureEnoughWorkersLockRequired(
     BaseScopedWorkersExecutor* base_executor) {
   // Don't do anything if the thread group isn't started.
-  if (max_tasks_ == 0)
+  if (max_tasks_ == 0 || UNLIKELY(join_for_testing_started_))
     return;
 
   ScopedWorkersExecutor* executor =

base/task/thread_pool/thread_group_impl.h

@@ -20,7 +20,6 @@
 #include "base/memory/ref_counted.h"
 #include "base/optional.h"
 #include "base/strings/string_piece.h"
-#include "base/synchronization/atomic_flag.h"
 #include "base/synchronization/condition_variable.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task/thread_pool/task.h"
@@ -329,10 +328,8 @@ class BASE_EXPORT ThreadGroupImpl : public ThreadGroup {
   std::unique_ptr<ConditionVariable> num_workers_cleaned_up_for_testing_cv_
       GUARDED_BY(lock_);
 
-#if DCHECK_IS_ON()
   // Set at the start of JoinForTesting().
-  AtomicFlag join_for_testing_started_;
-#endif
+  bool join_for_testing_started_ GUARDED_BY(lock_) = false;
 
   // ThreadPool.DetachDuration.[thread group name] histogram. Intentionally
   // leaked.

base/task/thread_pool/thread_pool_impl_unittest.cc

@@ -466,9 +466,7 @@ TEST_P(ThreadPoolImplTestAllTraitsExecutionModes,
 
 // Verify that posting tasks after the thread pool was destroyed fails but
 // doesn't crash.
-// crbug.com/1010145: disabled due to failure.
-TEST_P(ThreadPoolImplTestAllTraitsExecutionModes,
-       DISABLED_PostTaskAfterDestroy) {
+TEST_P(ThreadPoolImplTestAllTraitsExecutionModes, PostTaskAfterDestroy) {
   StartThreadPool();
 
   auto task_runner = CreateTaskRunnerAndExecutionMode(