base/threading: Lazy thread startup for pthreads

base/threading/platform_thread_posix.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/platform_thread.h"
 
 #include <errno.h>
 #include <pthread.h>
 #include <sched.h>
 #include <sys/resource.h>
@@ skipping 18 matching lines @@
 
 void InitThreading();
 void InitOnThread();
 void TerminateOnThread();
 size_t GetDefaultThreadStackSize(const pthread_attr_t& attributes);
 
 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)
     base::ThreadRestrictions::SetSingletonAllowed(false);
 
   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());
 
   delegate->ThreadMain();
 
   ThreadIdNameManager::GetInstance()->RemoveName(
       PlatformThread::CurrentHandle().platform_handle(),
       PlatformThread::CurrentId());
 
   base::TerminateOnThread();
   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)
     stack_size = base::GetDefaultThreadStackSize(attributes);
 
   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;
 }
 
 }  // namespace
 
 // static
 PlatformThreadId PlatformThread::CurrentId() {
   // Pthreads doesn't have the concept of a thread ID, so we have to reach down
   // into the kernel.
 #if defined(OS_MACOSX)
@@ skipping 46 matching lines @@
 
 // static
 const char* PlatformThread::GetName() {
   return ThreadIdNameManager::GetInstance()->GetName(CurrentId());
 }
 
 // static
 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);
 }
 
 // static
 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;
 }
 
 // static
 void PlatformThread::Join(PlatformThreadHandle thread_handle) {
   // Joining another thread may block the current thread for a long time, since
   // the thread referred to by |thread_handle| may still be running long-lived /
   // blocking tasks.
@@ skipping 49 matching lines @@
     return ThreadPriority::NORMAL;
   }
 
   return internal::NiceValueToThreadPriority(nice_value);
 #endif  // !defined(OS_NACL)
 }
 
 #endif  // !defined(OS_MACOSX)
 
 }  // namespace base