Do not block in SimpleThread::Start().

base/threading/simple_thread.cc

Index: base/threading/simple_thread.cc
diff --git a/base/threading/simple_thread.cc b/base/threading/simple_thread.cc
index 9eb443afab1b5d5dcb3dcd7427f8c4a1faacbb1b..2f160f06b1804cc38ead96b70c0d032d4f2539da 100644
--- a/base/threading/simple_thread.cc
+++ b/base/threading/simple_thread.cc
@@ -18,17 +18,25 @@ SimpleThread::SimpleThread(const std::string& name_prefix,
                            const Options& options)
     : name_prefix_(name_prefix),
       options_(options),
-      event_(WaitableEvent::ResetPolicy::MANUAL,
-             WaitableEvent::InitialState::NOT_SIGNALED) {}
+      tid_set_event_(WaitableEvent::ResetPolicy::MANUAL,
+                     WaitableEvent::InitialState::NOT_SIGNALED) {}
 
 SimpleThread::~SimpleThread() {
-  DCHECK(HasBeenStarted()) << "SimpleThread was never started.";
-  DCHECK(!options_.joinable || HasBeenJoined())
+#if DCHECK_IS_ON()
+  DCHECK(has_been_started_) << "SimpleThread was never started.";
+  DCHECK(!options_.joinable || has_been_joined_)
       << "Joinable SimpleThread destroyed without being Join()ed.";
+#endif
 }
 
 void SimpleThread::Start() {
-  DCHECK(!HasBeenStarted()) << "Tried to Start a thread multiple times.";
+#if DCHECK_IS_ON()
+  DCHECK(!has_been_started_) << "Tried to Start a thread multiple times.";
+
+  // Set |has_been_started_| before creating the thread so that the thread's
+  // main function sees it. Otherwise, the thread's main function couldn't

[gab, 2017/02/06 20:29:10]

couldn't... ?

[fdoray, 2017/02/06 20:46:48]

Done.

+  has_been_started_ = true;
+#endif
   bool success =
       options_.joinable
           ? PlatformThread::CreateWithPriority(options_.stack_size, this,
@@ -36,35 +44,36 @@ void SimpleThread::Start() {
           : PlatformThread::CreateNonJoinableWithPriority(
                 options_.stack_size, this, options_.priority);
   DCHECK(success);
-  ThreadRestrictions::ScopedAllowWait allow_wait;

[gab, 2017/02/06 20:29:10]

Remove 

friend class SimpleThread;

from thread_restrictions.h

[fdoray, 2017/02/06 20:46:48]

Done.

-  event_.Wait();  // Wait for the thread to complete initialization.
 }

[gab, 2017/02/06 20:29:10]

Add after DCHECK(success):

// No member access after creating the thread, |this| can be deleted at any
point after invoking Run() on non-joinable threads.

[fdoray, 2017/02/06 20:46:48]

Done.

 
 void SimpleThread::Join() {
+#if DCHECK_IS_ON()
   DCHECK(options_.joinable) << "A non-joinable thread can't be joined.";
-  DCHECK(HasBeenStarted()) << "Tried to Join a never-started thread.";
-  DCHECK(!HasBeenJoined()) << "Tried to Join a thread multiple times.";
+  DCHECK(has_been_started_) << "Tried to Join a never-started thread.";
+  DCHECK(!has_been_joined_) << "Tried to Join a thread multiple times.";
+#endif
   PlatformThread::Join(thread_);
   thread_ = PlatformThreadHandle();
-  joined_ = true;
+#if DCHECK_IS_ON()
+  has_been_joined_ = true;
+#endif
 }
 
-bool SimpleThread::HasBeenStarted() {
-  ThreadRestrictions::ScopedAllowWait allow_wait;

[gab, 2017/02/06 20:29:10]

Nice, always thought it made no sense to blindly allow it for the caller.

[fdoray, 2017/02/06 20:46:48]

Acknowledged.

-  return event_.IsSignaled();
+PlatformThreadId SimpleThread::GetTid() const {
+  tid_set_event_.Wait();
+  return tid_;
 }
 
 void SimpleThread::ThreadMain() {
   tid_ = PlatformThread::CurrentId();
+  tid_set_event_.Signal();
+
   // Construct our full name of the form "name_prefix_/TID".
   std::string name(name_prefix_);
   name.push_back('/');
   name.append(IntToString(tid_));
   PlatformThread::SetName(name);
 
-  // We've initialized our new thread, signal that we're done to Start().
-  event_.Signal();
-
   Run();
 }