base/threading: fix a potential race issue on start_event_

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/lazy_instance.h"
 #include "base/location.h"
 #include "base/synchronization/waitable_event.h"
@@ skipping 48 matching lines @@
 #if defined(OS_WIN)
       com_status_(NONE),
 #endif
       stopping_(false),
       running_(false),
       thread_(0),
       id_(kInvalidThreadId),
       id_event_(true, false),
       message_loop_(nullptr),
       message_loop_timer_slack_(TIMER_SLACK_NONE),
-      name_(name) {
+      name_(name),
+      start_event_(false, false) {
 }
 
 Thread::~Thread() {
   Stop();
 }
 
 bool Thread::Start() {
   Options options;
 #if defined(OS_WIN)
   if (com_status_ == STA)
@@ skipping 16 matching lines @@
   SetThreadWasQuitProperly(false);
 
   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;
   scoped_ptr<MessageLoop> message_loop = MessageLoop::CreateUnbound(
       type, options.message_pump_factory);
   message_loop_ = message_loop.get();
-  start_event_.reset(new WaitableEvent(false, false));
+  start_event_.Reset();
 
   // Hold the thread_lock_ while starting a new thread, so that we can make sure
   // that thread_ is populated before the newly created thread accesses it.
   {
     AutoLock lock(thread_lock_);
     if (!PlatformThread::CreateWithPriority(options.stack_size, this, &thread_,
                                             options.priority)) {
       DLOG(ERROR) << "failed to create thread";
       message_loop_ = nullptr;
-      start_event_.reset();
       return false;
     }
   }
 
   // The ownership of message_loop is managemed by the newly created thread
   // within the ThreadMain.
   ignore_result(message_loop.release());
 
   DCHECK(message_loop_);
   return true;
 }
 
 bool Thread::StartAndWaitForTesting() {
   bool result = Start();
   if (!result)
     return false;
   WaitUntilThreadStarted();
   return true;
 }
 
-bool Thread::WaitUntilThreadStarted() {
-  if (!start_event_)
+bool Thread::WaitUntilThreadStarted() const {
+  if (!message_loop_)
     return false;
   base::ThreadRestrictions::ScopedAllowWait allow_wait;
-  start_event_->Wait();
+  start_event_.Wait();
   return true;
 }
 
 void Thread::Stop() {
-  if (!start_event_)
+  AutoLock lock(thread_lock_);
+  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 NULL message_loop_ on exit.
+  // The thread should nullify message_loop_ on exit.
   DCHECK(!message_loop_);
 
-  // The thread no longer needs to be joined.
-  start_event_.reset();
-
   stopping_ = false;
 }
 
 void Thread::StopSoon() {
   // We should only be called on the same thread that started us.
 
   DCHECK_NE(GetThreadId(), PlatformThread::CurrentId());
 
   if (stopping_ || !message_loop_)
     return;
@@ skipping 66 matching lines @@
 #endif
 
   // Let the thread do extra initialization.
   Init();
 
   {
     AutoLock lock(running_lock_);
     running_ = true;
   }
 
-  start_event_->Signal();
+  start_event_.Signal();
 
   Run(message_loop_);
 
   {
     AutoLock lock(running_lock_);
     running_ = false;
   }
 
   // Let the thread do extra cleanup.
   CleanUp();
 
 #if defined(OS_WIN)
   com_initializer.reset();
 #endif
 
   // Assert that MessageLoop::Quit was called by ThreadQuitHelper.
   DCHECK(GetThreadWasQuitProperly());
 
   // We can't receive messages anymore.
   // (The message loop is destructed at the end of this block)
-  message_loop_ = NULL;
+  message_loop_ = nullptr;
 }
 
 }  // namespace base