Use CreateThread instead of _beginthreadex. This should still be safe, and w...

base/platform_thread_win.cc

 // Copyright (c) 2006-2008 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/platform_thread.h"
 
 #include <process.h>

[cpu_(ooo_6.6-7.5), 2008/11/04 03:07:58]

remove this include

 
 #include "base/logging.h"
 #include "base/win_util.h"
 
 namespace {
 
 // The information on how to set the thread name comes from
 // a MSDN article: http://msdn2.microsoft.com/en-us/library/xcb2z8hs.aspx
 const DWORD kVCThreadNameException = 0x406D1388;
 
 typedef struct tagTHREADNAME_INFO {
   DWORD dwType;  // Must be 0x1000.
   LPCSTR szName;  // Pointer to name (in user addr space).
   DWORD dwThreadID;  // Thread ID (-1=caller thread).
   DWORD dwFlags;  // Reserved for future use, must be zero.
 } THREADNAME_INFO;
 
-unsigned __stdcall ThreadFunc(void* closure) {
+DWORD __stdcall ThreadFunc(void* closure) {
   PlatformThread::Delegate* delegate =
       static_cast<PlatformThread::Delegate*>(closure);
   delegate->ThreadMain(); 
   return NULL;
 }
 
 }  // namespace
 
 // static
 int PlatformThread::CurrentId() {
   return GetCurrentThreadId();
 }
 
 // static
 void PlatformThread::YieldCurrentThread() {
   ::Sleep(0);
 }
 
 // static
 void PlatformThread::Sleep(int duration_ms) {
   ::Sleep(duration_ms);
 }
 
 // static
 void PlatformThread::SetName(const char* name) {
   // The debugger needs to be around to catch the name in the exception.  If
   // there isn't a debugger, we are just needlessly throwing an exception.
-  if (!::IsDebuggerPresent())
+  if (0 && !::IsDebuggerPresent())

[Peter Kasting, 2008/11/03 22:09:32]

Did you mean this change to be part of this CL?  Looks like a local
modification.

     return;
 
   THREADNAME_INFO info;
   info.dwType = 0x1000;
   info.szName = name;
   info.dwThreadID = CurrentId();
   info.dwFlags = 0;
 
   __try {
     RaiseException(kVCThreadNameException, 0, sizeof(info)/sizeof(DWORD),
                    reinterpret_cast<DWORD_PTR*>(&info));
   } __except(EXCEPTION_CONTINUE_EXECUTION) {
   }
 }
 
 // static
 bool PlatformThread::Create(size_t stack_size, Delegate* delegate,
                             PlatformThreadHandle* thread_handle) {
   unsigned int flags = 0;
   if (stack_size > 0 && win_util::GetWinVersion() >= win_util::WINVERSION_XP) {
     flags = STACK_SIZE_PARAM_IS_A_RESERVATION;
   } else {
     stack_size = 0;
   }
 
-  *thread_handle = reinterpret_cast<PlatformThreadHandle>(_beginthreadex(
-      NULL, stack_size, ThreadFunc, delegate, flags, NULL));
+  // Using CreateThread here vs _beginthreadex makes thread creation a bit
+  // faster and doesn't require holding the loader lock.  Our code will have

[tommi (sloooow) - chröme, 2008/11/30 05:29:33]

hi... really late, drive by comment here.  I noticed this line "doesn't require
holding the loader lock" for CreateThread. Actually even with CreateThread when
a thread starts, it does acquire the loader lock in order to call
DllMain(DLL_THREAD_ATTACH) for all dlls that haven't opted out of those calls.
just an fyi.
the _beginthreadex() call adds a crt thread stub that sets up TLS stuff.  I
doubt that chrome depends on it though, so the change is fine by me.  Just an
fyi on the comment in the code.

+  // to work running on CreateThread() threads anyway, since we run code on
+  // the Windows thread pool, etc.  For some background on the difference:
+  //   http://www.microsoft.com/msj/1099/win32/win321099.aspx

[M-A Ruel, 2008/11/03 22:22:11]

Add these links too:
http://msdn.microsoft.com/en-us/library/kdzttdcb.aspx
http://msdn.microsoft.com/en-us/library/7t9ha0zh.aspx

+  *thread_handle = CreateThread(
+      NULL, stack_size, ThreadFunc, delegate, flags, NULL);
   return *thread_handle != NULL;
 }
 
 // static
 void PlatformThread::Join(PlatformThreadHandle thread_handle) {
   DCHECK(thread_handle);
 
   // Wait for the thread to exit.  It should already have terminated but make
   // sure this assumption is valid.
   DWORD result = WaitForSingleObject(thread_handle, INFINITE);
   DCHECK_EQ(WAIT_OBJECT_0, result);
 
   CloseHandle(thread_handle);
 }