base/threading: Lazy thread startup for pthreads

base/threading/platform_thread_posix.cc

Index: base/threading/platform_thread_posix.cc
diff --git a/base/threading/platform_thread_posix.cc b/base/threading/platform_thread_posix.cc
index 44b691c1d638bd4fde69dbbe5b83adfc6f63092e..c6c174355be3a16b661248b4da8430e2b338e635 100644
--- a/base/threading/platform_thread_posix.cc
+++ b/base/threading/platform_thread_posix.cc
@@ -36,23 +36,17 @@ namespace {
 
 struct ThreadParams {
   ThreadParams()
-      : delegate(NULL),
-        joinable(false),
-        priority(ThreadPriority::NORMAL),
-        handle(NULL),
-        handle_set(false, false) {
-  }
+      : delegate(NULL), joinable(false), priority(ThreadPriority::NORMAL) {}
 
   PlatformThread::Delegate* delegate;
   bool joinable;
   ThreadPriority priority;
-  PlatformThreadHandle* handle;
-  WaitableEvent handle_set;

[Lei Zhang, 2015/07/28 06:32:55]

I think you can remove #include base/synchronization/waitable_event.h after
this.

[Takashi Toyoshima, 2015/07/28 11:13:43]

Done.

 };
 
 void* ThreadFunc(void* params) {
+  CHECK(params);

[Lei Zhang, 2015/07/28 06:32:55]

Is this needed? If the CHECK() fails, we'll just crash a few lines down anyway
and know something is wrong.

[Takashi Toyoshima, 2015/07/28 11:13:43]

Agreed. I removed this redundant check.

   base::InitOnThread();
-  ThreadParams* thread_params = static_cast<ThreadParams*>(params);
+  scoped_ptr<ThreadParams> thread_params(static_cast<ThreadParams*>(params));
 
   PlatformThread::Delegate* delegate = thread_params->delegate;
   if (!thread_params->joinable)
@@ -61,11 +55,6 @@ void* ThreadFunc(void* params) {
   if (thread_params->priority != ThreadPriority::NORMAL)
     PlatformThread::SetCurrentThreadPriority(thread_params->priority);
 
-  // Stash the id in the handle so the calling thread has a complete
-  // handle, and unblock the parent thread.
-  *(thread_params->handle) = PlatformThreadHandle(pthread_self());
-  thread_params->handle_set.Signal();
-
   ThreadIdNameManager::GetInstance()->RegisterThread(
       PlatformThread::CurrentHandle().platform_handle(),
       PlatformThread::CurrentId());
@@ -80,21 +69,21 @@ void* ThreadFunc(void* params) {
   return NULL;
 }
 
-bool CreateThread(size_t stack_size, bool joinable,
+bool CreateThread(size_t stack_size,
+                  bool joinable,
                   PlatformThread::Delegate* delegate,
                   PlatformThreadHandle* thread_handle,
                   ThreadPriority priority) {
+  CHECK(thread_handle);

[Lei Zhang, 2015/07/28 06:32:55]

Will a DCHECK() suffice here?

[Takashi Toyoshima, 2015/07/28 11:13:43]

Agreed. This check should be safe to ignore in release builds.

   base::InitThreading();
 
-  bool success = false;
   pthread_attr_t attributes;
   pthread_attr_init(&attributes);
 
   // Pthreads are joinable by default, so only specify the detached
   // attribute if the thread should be non-joinable.
-  if (!joinable) {
+  if (!joinable)
     pthread_attr_setdetachstate(&attributes, PTHREAD_CREATE_DETACHED);
-  }
 
   // Get a better default if available.
   if (stack_size == 0)
@@ -103,33 +92,28 @@ bool CreateThread(size_t stack_size, bool joinable,
   if (stack_size > 0)
     pthread_attr_setstacksize(&attributes, stack_size);
 
-  ThreadParams params;
-  params.delegate = delegate;
-  params.joinable = joinable;
-  params.priority = priority;
-  params.handle = thread_handle;
+  scoped_ptr<ThreadParams> params(new ThreadParams);
+  params->delegate = delegate;
+  params->joinable = joinable;
+  params->priority = priority;
 
   pthread_t handle;
-  int err = pthread_create(&handle,
-                           &attributes,
-                           ThreadFunc,
-                           &params);
-  success = !err;
-  if (!success) {
+  int err =
+      pthread_create(&handle, &attributes, ThreadFunc, params.get());
+  bool success = !err;
+  if (success) {
+    // ThreadParams should be deleted on the created thread after used.
+    ignore_result(params.release());
+  } else {
     // Value of |handle| is undefined if pthread_create fails.
     handle = 0;
     errno = err;
     PLOG(ERROR) << "pthread_create";
   }
+  *thread_handle = PlatformThreadHandle(handle);
 
   pthread_attr_destroy(&attributes);
 
-  // Don't let this call complete until the thread id
-  // is set in the handle.
-  if (success)
-    params.handle_set.Wait();
-  CHECK_EQ(handle, thread_handle->platform_handle());
-
   return success;
 }
 
@@ -196,7 +180,6 @@ const char* PlatformThread::GetName() {
 bool PlatformThread::CreateWithPriority(size_t stack_size, Delegate* delegate,
                                         PlatformThreadHandle* thread_handle,
                                         ThreadPriority priority) {
-  base::ThreadRestrictions::ScopedAllowWait allow_wait;
   return CreateThread(stack_size, true,  // joinable thread
                       delegate, thread_handle, priority);
 }
@@ -205,7 +188,6 @@ bool PlatformThread::CreateWithPriority(size_t stack_size, Delegate* delegate,
 bool PlatformThread::CreateNonJoinable(size_t stack_size, Delegate* delegate) {
   PlatformThreadHandle unused;
 
-  base::ThreadRestrictions::ScopedAllowWait allow_wait;
   bool result = CreateThread(stack_size, false /* non-joinable thread */,
                              delegate, &unused, ThreadPriority::NORMAL);
   return result;