From eaea4278689ac7f2919b60d65eda33cd4aa7cad2 Mon Sep 17 00:00:00 2001
From: drbasic <drbasic@yandex-team.ru>
Date: Thu, 11 Sep 2014 21:19:31 -0700
Subject: Fix potential race-condition _beginthread

Sometimes ppapi tests fails due to race condition.
It's safer to use _beginthreadex than _beginthread. If the thread
that's generated by _beginthread exits quickly, the handle that's
returned to the caller of _beginthread might be invalid or point to
another thread. However, the handle that's returned by _beginthreadex
has to be closed by the caller of _beginthreadex, so it is guaranteed
to be a valid handle if _beginthreadex did not return an error.

R=bbudge@chromium.org
BUG=

Review URL: https://codereview.chromium.org/547543002

Cr-Commit-Position: refs/heads/master@{#294541}
---
 ppapi/tests/pp_thread.h          | 48 ++++++++++++++++++++++++++--------------
 ppapi/tests/test_case.cc         |  2 +-
 ppapi/tests/test_post_message.cc |  2 +-
 3 files changed, 34 insertions(+), 18 deletions(-)

(limited to 'ppapi/tests')

diff --git a/ppapi/tests/pp_thread.h b/ppapi/tests/pp_thread.h
index c4af8f1..c80ffa1 100644
--- a/ppapi/tests/pp_thread.h
+++ b/ppapi/tests/pp_thread.h
@@ -33,18 +33,22 @@
  * used in ppapi/tests, so is not part of the published API.
  */
 
+typedef void (PP_ThreadFunction)(void* data);
+
 #if defined(PPAPI_POSIX)
-typedef pthread_t PP_ThreadType;
+typedef pthread_t PP_Thread;
 #elif defined(PPAPI_OS_WIN)
-typedef uintptr_t PP_ThreadType;
+struct PP_Thread {
+  HANDLE handle;
+  PP_ThreadFunction* thread_func;
+  void* thread_arg;
+};
 #endif
 
-typedef void (PP_ThreadFunction)(void* data);
-
-PP_INLINE bool PP_CreateThread(PP_ThreadType* thread,
+PP_INLINE bool PP_CreateThread(PP_Thread* thread,
                                PP_ThreadFunction function,
                                void* thread_arg);
-PP_INLINE void PP_JoinThread(PP_ThreadType thread);
+PP_INLINE void PP_JoinThread(PP_Thread thread);
 
 #if defined(PPAPI_POSIX)
 /* Because POSIX thread functions return void* and Windows thread functions do
@@ -65,7 +69,7 @@ PP_INLINE void* PP_POSIXThreadFunctionThunk(void* posix_thread_arg) {
   return NULL;
 }
 
-PP_INLINE bool PP_CreateThread(PP_ThreadType* thread,
+PP_INLINE bool PP_CreateThread(PP_Thread* thread,
                                PP_ThreadFunction function,
                                void* thread_arg) {
   PP_ThreadFunctionArgWrapper* arg_wrapper =
@@ -78,27 +82,39 @@ PP_INLINE bool PP_CreateThread(PP_ThreadType* thread,
                          arg_wrapper) == 0);
 }
 
-PP_INLINE void PP_JoinThread(PP_ThreadType thread) {
+PP_INLINE void PP_JoinThread(PP_Thread thread) {
   void* exit_status;
   pthread_join(thread, &exit_status);
 }
 
 #elif defined(PPAPI_OS_WIN)
-typedef DWORD (PP_WindowsThreadFunction)(void* data);
 
-PP_INLINE bool PP_CreateThread(PP_ThreadType* thread,
+PP_INLINE unsigned __stdcall PP_WindowsThreadFunction(void* param) {
+  PP_Thread* thread = reinterpret_cast<PP_Thread*>(param);
+  thread->thread_func(thread->thread_arg);
+  return 0;
+}
+
+PP_INLINE bool PP_CreateThread(PP_Thread* thread,
                                PP_ThreadFunction function,
                                void* thread_arg) {
   if (!thread)
     return false;
-  *thread = ::_beginthread(function,
-                           0,  /* Use default stack size. */
-                           thread_arg);
-  return (*thread != NULL);
+  thread->thread_func = function;
+  thread->thread_arg = thread_arg;
+  uintptr_t raw_handle = ::_beginthreadex(NULL,
+                                          0,  /* Use default stack size. */
+                                          &PP_WindowsThreadFunction,
+                                          thread,
+                                          0,
+                                          NULL);
+  thread->handle = reinterpret_cast<HANDLE>(raw_handle);
+  return (thread->handle != NULL);
 }
 
-PP_INLINE void PP_JoinThread(PP_ThreadType thread) {
-  ::WaitForSingleObject((HANDLE)thread, INFINITE);
+PP_INLINE void PP_JoinThread(PP_Thread thread) {
+  ::WaitForSingleObject(thread.handle, INFINITE);
+  ::CloseHandle(thread.handle);
 }
 
 #endif
diff --git a/ppapi/tests/test_case.cc b/ppapi/tests/test_case.cc
index 52ec306..a257556 100644
--- a/ppapi/tests/test_case.cc
+++ b/ppapi/tests/test_case.cc
@@ -251,7 +251,7 @@ void TestCase::RunOnThreadInternal(
     void (*thread_func)(void*),
     void* thread_param,
     const PPB_Testing_Private* testing_interface) {
-  PP_ThreadType thread;
+  PP_Thread thread;
   PP_CreateThread(&thread, thread_func, thread_param);
   // Run a message loop so pepper calls can be dispatched. The background
   // thread will set result_ and make us Quit when it's done.
diff --git a/ppapi/tests/test_post_message.cc b/ppapi/tests/test_post_message.cc
index 6a415bb..8e7be05 100644
--- a/ppapi/tests/test_post_message.cc
+++ b/ppapi/tests/test_post_message.cc
@@ -869,7 +869,7 @@ std::string TestPostMessage::TestNonMainThread() {
   // times.  For good measure, call postMessage from the main thread
   // kMessagesToSendPerThread times. At the end, we make sure we got all the
   // values we expected.
-  PP_ThreadType threads[kThreadsToRun];
+  PP_Thread threads[kThreadsToRun];
   for (int32_t i = 0; i < kThreadsToRun; ++i) {
     // Set up a thread to send a value of i.
     void* arg = new InvokePostMessageThreadArg(instance_, pp::Var(i));
-- 
cgit v1.1