Re-enable Thread::Stop() sequence checks and consequently remove |thread_lock|.

base/threading/thread.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/thread.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/location.h"
@@ skipping 77 matching lines @@
   MessageLoop::Type type = options.message_loop_type;
   if (!options.message_pump_factory.is_null())
     type = MessageLoop::TYPE_CUSTOM;
 
   message_loop_timer_slack_ = options.timer_slack;
   std::unique_ptr<MessageLoop> message_loop_owned =
       MessageLoop::CreateUnbound(type, options.message_pump_factory);
   message_loop_ = message_loop_owned.get();
   start_event_.Reset();
 
-  // Hold |thread_lock_| while starting the new thread to synchronize with
-  // Stop() while it's not guaranteed to be sequenced (until crbug/629139 is
-  // fixed).
-  {
-    AutoLock lock(thread_lock_);
-    if (!PlatformThread::CreateWithPriority(options.stack_size, this, &thread_,
-                                            options.priority)) {
-      DLOG(ERROR) << "failed to create thread";
-      message_loop_ = nullptr;
-      return false;
-    }
+  if (!PlatformThread::CreateWithPriority(options.stack_size, this, &thread_,
+                                          options.priority)) {
+    DLOG(ERROR) << "failed to create thread";
+    message_loop_ = nullptr;
+    return false;
   }
 
   // The ownership of |message_loop_| is managed by the newly created thread
   // within the ThreadMain.
   ignore_result(message_loop_owned.release());
 
   DCHECK(message_loop_);
   return true;
 }
 
 bool Thread::StartAndWaitForTesting() {
   DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
   bool result = Start();
   if (!result)
     return false;
   WaitUntilThreadStarted();
   return true;
 }
 
 bool Thread::WaitUntilThreadStarted() const {
   DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
   if (!message_loop_)
     return false;
   base::ThreadRestrictions::ScopedAllowWait allow_wait;
   start_event_.Wait();
   return true;
 }
 
 void Thread::Stop() {
-  // TODO(gab): Fix improper usage of this API (http://crbug.com/629139) and
-  // enable this check, until then synchronization with Start() via
-  // |thread_lock_| is required...
-  // DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
-  AutoLock lock(thread_lock_);
+  DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
 
   if (thread_.is_null())
     return;
 
   StopSoon();
 
   // Wait for the thread to exit.
   //
   // TODO(darin): Unfortunately, we need to keep |message_loop_| around until
   // the thread exits.  Some consumers are abusing the API.  Make them stop.
   //
   PlatformThread::Join(thread_);
   thread_ = base::PlatformThreadHandle();
 
   // The thread should nullify |message_loop_| on exit (note: Join() adds an
   // implicit memory barrier and no lock is thus required for this check).
   DCHECK(!message_loop_);
 
   stopping_ = false;
 }
 
 void Thread::StopSoon() {
-  // TODO(gab): Fix improper usage of this API (http://crbug.com/629139) and
-  // enable this check.
-  // DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
 
   if (stopping_ || !message_loop_)
     return;
 
   stopping_ = true;
   task_runner()->PostTask(
       FROM_HERE, base::Bind(&Thread::ThreadQuitHelper, Unretained(this)));
 }
 
 PlatformThreadId Thread::GetThreadId() const {
@@ skipping 106 matching lines @@
   run_loop_ = nullptr;
 }
 
 void Thread::ThreadQuitHelper() {
   DCHECK(run_loop_);
   run_loop_->QuitWhenIdle();
   SetThreadWasQuitProperly(true);
 }
 
 }  // namespace base