Add |joinable| to Thread::Options

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"
+#include "base/logging.h"
 #include "base/run_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/threading/thread_id_name_manager.h"
 #include "base/threading/thread_local.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include "base/win/scoped_com_initializer.h"
 #endif
 
 namespace base {
 
 namespace {
 
 // We use this thread-local variable to record whether or not a thread exited
 // because its Stop method was called.  This allows us to catch cases where
 // MessageLoop::QuitWhenIdle() is called directly, which is unexpected when
 // using a Thread to setup and run a MessageLoop.
-base::LazyInstance<base::ThreadLocalBoolean> lazy_tls_bool =
+base::LazyInstance<base::ThreadLocalBoolean>::Leaky lazy_tls_bool =
     LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 Thread::Options::Options() = default;
 
 Thread::Options::Options(MessageLoop::Type type, size_t size)
     : message_loop_type(type), stack_size(size) {}
 
 Thread::Options::Options(const Options& other) = default;
 
 Thread::Options::~Options() = default;
 
 Thread::Thread(const std::string& name)
     : id_event_(WaitableEvent::ResetPolicy::MANUAL,
                 WaitableEvent::InitialState::NOT_SIGNALED),
       name_(name),
       start_event_(WaitableEvent::ResetPolicy::MANUAL,
                    WaitableEvent::InitialState::NOT_SIGNALED) {
   // Only bind the sequence on Start(): the state is constant between
   // construction and Start() and it's thus valid for Start() to be called on
   // another sequence as long as every other operation is then performed on that
   // sequence.
   owning_sequence_checker_.DetachFromSequence();
 }
 
 Thread::~Thread() {
   Stop();
+
+  // ThreadMain() must have returned before this Thread instance is destroyed
+  // or use-after-frees could ensue.
+  DCHECK(!thread_main_exited_.get() || thread_main_exited_->IsSet());

[danakj, 2016/07/28 19:07:40]

no .get()

[gab, 2016/07/28 21:40:15]

I intentionally added the .get()'s here and below because

DCHECK(!thread_main_exited_ || thread_main_exited_->IsSet());

felt ackward (i.e. "not foo or foo is set" sounds like testing opposites...).

 }
 
 bool Thread::Start() {
   DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
 
   Options options;
 #if defined(OS_WIN)
   if (com_status_ == STA)
     options.message_loop_type = MessageLoop::TYPE_UI;
 #endif
   return StartWithOptions(options);
 }
 
 bool Thread::StartWithOptions(const Options& options) {
   DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
   DCHECK(!message_loop_);
   DCHECK(!IsRunning());
+  DCHECK(!stopping_) << "Starting a non-joinable thread a second time? That's "
+                     << "not allowed!";
 #if defined(OS_WIN)
   DCHECK((com_status_ != STA) ||
       (options.message_loop_type == MessageLoop::TYPE_UI));
 #endif
 
   // Reset |id_| here to support restarting the thread.
   id_event_.Reset();
   id_ = kInvalidThreadId;
 
+  if (!thread_main_exited_.get()) {

[danakj, 2016/07/28 19:07:40]

no .get()

[gab, 2016/07/28 21:40:15]

Similarly here:

if (!thread_main_exited_)

feels like it's testing a boolean whereas the absence of the pointer means the
opposite. So I feel like the .get() makes this more obvious and draws attention
to the comment on the member variable explaining it.

+    thread_main_exited_.reset(new AtomicFlag);
+  } else {
+    // It's safe to reset |thread_main_exited_| on sequence when the underlying
+    // thread isn't running.
+    thread_main_exited_->UnsafeReset();
+  }
+
   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;
   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)) {
+    bool success =
+        options.joinable
+            ? PlatformThread::CreateWithPriority(options.stack_size, this,
+                                                 &thread_, options.priority)
+            : PlatformThread::CreateNonJoinableWithPriority(
+                  options.stack_size, this, options.priority);
+    if (!success) {
       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());
 
@@ skipping 19 matching lines @@
   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_);
 
+  StopSoon();
+
+  // Can't join if the |thread_| is either already gone or is non-joinable.
   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());
 
   if (stopping_ || !message_loop_)
     return;
 
   stopping_ = true;
+
+  if (using_external_message_loop_) {
+    // Setting |stopping_| to true above should have been sufficient for this
+    // thread to be considered "stopped" per it having never set its |running_|
+    // bit by lack of its own ThreadMain.
+    DCHECK(!IsRunning());
+    message_loop_ = nullptr;
+    return;
+  }
+
   task_runner()->PostTask(
       FROM_HERE, base::Bind(&Thread::ThreadQuitHelper, Unretained(this)));
 }
 
 PlatformThreadId Thread::GetThreadId() const {
   // If the thread is created but not started yet, wait for |id_| being ready.
   base::ThreadRestrictions::ScopedAllowWait allow_wait;
   id_event_.Wait();
   return id_;
 }
@@ skipping 29 matching lines @@
 
 // static
 bool Thread::GetThreadWasQuitProperly() {
   bool quit_properly = true;
 #ifndef NDEBUG
   quit_properly = lazy_tls_bool.Pointer()->Get();
 #endif
   return quit_properly;
 }
 
+void Thread::SetMessageLoop(MessageLoop* message_loop) {
+  DCHECK(owning_sequence_checker_.CalledOnValidSequencedThread());
+
+  // Setting |message_loop_| should suffice for this thread to be considered
+  // as "running", until Stop() is invoked.
+  DCHECK(!IsRunning());
+  message_loop_ = message_loop;
+  DCHECK(IsRunning());
+
+  using_external_message_loop_ = true;
+}
+
 void Thread::ThreadMain() {
   // First, make GetThreadId() available to avoid deadlocks. It could be called
   // any place in the following thread initialization code.
   id_ = PlatformThread::CurrentId();
   DCHECK_NE(kInvalidThreadId, id_);
   id_event_.Signal();
 
   // Complete the initialization of our Thread object.
   PlatformThread::SetName(name_.c_str());
   ANNOTATE_THREAD_NAME(name_.c_str());  // Tell the name to race detector.
@@ skipping 43 matching lines @@
     // Assert that RunLoop::QuitWhenIdle was called by ThreadQuitHelper. Don't
     // check for custom message pumps, because their shutdown might not allow
     // this.
     DCHECK(GetThreadWasQuitProperly());
   }
 
   // We can't receive messages anymore.
   // (The message loop is destructed at the end of this block)
   message_loop_ = nullptr;
   run_loop_ = nullptr;
+
+  thread_main_exited_->Set();
 }
 
 void Thread::ThreadQuitHelper() {
   DCHECK(run_loop_);
   run_loop_->QuitWhenIdle();
   SetThreadWasQuitProperly(true);
 }
 
 }  // namespace base