List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:July 22 2010 7:14am
Subject:bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3319) Bug#55434
View as plain text  
#At file:///export/home/didrik/repo/next-mr-bf-bug55434/ based on revid:jonathan.perkin@stripped

 3319 Tor Didriksen	2010-07-22
      Bug #55434 multithreaded gunit tests fail on windows
      
      On windows we need an open handle to the thread in order to join it.
     @ unittest/gunit/thread_utils.cc
        On windows: keep an open handle to a thread in order to join it safely.
        
        Use mysql_xxx() wrappers for pthread_xxx() functions.
     @ unittest/gunit/thread_utils.h
        Use mysql_mutex_t/mysql_cond_t rather than pthread_mutex_t/pthread_cond_t

    modified:
      unittest/gunit/thread_utils.cc
      unittest/gunit/thread_utils.h
=== modified file 'unittest/gunit/thread_utils.cc'
--- a/unittest/gunit/thread_utils.cc	2010-05-21 13:02:27 +0000
+++ b/unittest/gunit/thread_utils.cc	2010-07-22 07:14:24 +0000
@@ -15,14 +15,41 @@
 
 #include <gtest/gtest.h>
 #include "thread_utils.h"
+#include "mysql/psi/mysql_thread.h"
 
 namespace thread {
 
+/*
+  On windows we need an open handle to the thread in order to join it.
+  The three instances of Notification are used as follows:
+  - The main thread starts sub-thread, waits for m_thread_started.
+  - The main thread picks up the thread id, and does OpenThread to get a handle.
+  - The main thread tells the sub-thread that it can continue
+      (notifies m_thread_continue)
+  - The main thread waits until the sub-thread is actually running
+    (waits for m_thread_running) before destroying all objects.
+
+  All this is to work around a bug in pthread_create() in mysys/my_winthread,
+  which closes the thread handle (it has nowhere to store it).
+  When we later call pthread_join() we try to re-open a handle.
+  This will fail if the thread has already finished, and then the join will fail.
+ */
+class Thread_start_arg
+{
+public:
+  Thread_start_arg(Thread *thread) : m_thread(thread) {}
+
+  Notification m_thread_started;
+  Notification m_thread_continue;
+  Notification m_thread_running;
+  Thread *m_thread;
+};
+
 namespace {
 void *thread_start_routine(void *arg)
 {
-  Thread *thread= (Thread*) arg;
-  Thread::run_wrapper(thread);
+  Thread_start_arg *start_arg= (Thread_start_arg*) arg;
+  Thread::run_wrapper(start_arg);
   return NULL;
 }
 
@@ -43,51 +70,85 @@ Thread::~Thread()
 
 int Thread::start()
 {
-  return pthread_create(&m_thread_id, NULL, thread_start_routine, this);
+  Thread_start_arg start_arg(this);
+  const int retval=
+    pthread_create(&m_thread_id, NULL, thread_start_routine, &start_arg);
+
+  start_arg.m_thread_started.wait_for_notification();
+#ifdef __WIN__
+  m_thread_handle= OpenThread(SYNCHRONIZE, FALSE, m_thread_id);
+#endif
+  start_arg.m_thread_continue.notify();
+  start_arg.m_thread_running.wait_for_notification();
+  return retval;
 }
 
 
 void Thread::join()
 {
-  int failed= pthread_join(m_thread_id, NULL);
-  ASSERT_FALSE(failed);
+#ifdef __WIN__
+  DWORD ret= WaitForSingleObject(m_thread_handle, INFINITE);
+  if (ret != WAIT_OBJECT_0)
+  {
+    DWORD lasterror= GetLastError();
+    ADD_FAILURE()
+      << " could not join thread id " << m_thread_id
+      << " handle " << m_thread_handle
+      << " GetLastError: " << lasterror
+      ;
+  }
+  CloseHandle(m_thread_handle);
+#else
+  const int failed= pthread_join(m_thread_id, NULL);
+  if (failed)
+  {
+    ADD_FAILURE()
+      << " could not join thread id " << m_thread_id
+      << " failed: " << failed << " errno: " << errno
+      ;
+  }
+#endif
 }
 
 
-void Thread::run_wrapper(Thread *thread)
+void Thread::run_wrapper(Thread_start_arg *start_arg)
 {
   const my_bool error= my_thread_init();
   ASSERT_FALSE(error);
+  Thread *thread= start_arg->m_thread;
+  start_arg->m_thread_started.notify();
+  start_arg->m_thread_continue.wait_for_notification();
+  start_arg->m_thread_running.notify();
   thread->run();
   my_thread_end();
 }
 
 
-Mutex_lock::Mutex_lock(pthread_mutex_t *mutex) : m_mutex(mutex)
+Mutex_lock::Mutex_lock(mysql_mutex_t *mutex) : m_mutex(mutex)
 {
-  pthread_mutex_lock(m_mutex);
+  mysql_mutex_lock(m_mutex);
 }
 
 
 Mutex_lock::~Mutex_lock()
 {
-  const int failed= pthread_mutex_unlock(m_mutex);
+  const int failed= mysql_mutex_unlock(m_mutex);
   LOCAL_ASSERT_FALSE(failed);
 }
 
 
 Notification::Notification() : m_notified(FALSE)
 {
-  const int failed1= pthread_cond_init(&m_cond, NULL);
+  const int failed1= mysql_cond_init(0, &m_cond, NULL);
   LOCAL_ASSERT_FALSE(failed1);
-  const int failed2= pthread_mutex_init(&m_mutex, MY_MUTEX_INIT_FAST);
+  const int failed2= mysql_mutex_init(0, &m_mutex, MY_MUTEX_INIT_FAST);
   LOCAL_ASSERT_FALSE(failed2);
 }
 
 Notification::~Notification()
 {
-  pthread_mutex_destroy(&m_mutex);
-  pthread_cond_destroy(&m_cond);
+  mysql_mutex_destroy(&m_mutex);
+  mysql_cond_destroy(&m_cond);
 }
 
 bool Notification::has_been_notified()
@@ -101,7 +162,7 @@ void Notification::wait_for_notification
   Mutex_lock lock(&m_mutex);
   while (!m_notified)
   {
-    const int failed= pthread_cond_wait(&m_cond, &m_mutex);
+    const int failed= mysql_cond_wait(&m_cond, &m_mutex);
     ASSERT_FALSE(failed);
   }
 }
@@ -110,7 +171,7 @@ void Notification::notify()
 {
   Mutex_lock lock(&m_mutex);
   m_notified= TRUE;
-  const int failed= pthread_cond_broadcast(&m_cond);
+  const int failed= mysql_cond_broadcast(&m_cond);
   ASSERT_FALSE(failed);
 }
 

=== modified file 'unittest/gunit/thread_utils.h'
--- a/unittest/gunit/thread_utils.h	2010-03-19 11:46:14 +0000
+++ b/unittest/gunit/thread_utils.h	2010-07-22 07:14:24 +0000
@@ -21,6 +21,8 @@
 
 namespace thread {
 
+class Thread_start_arg;
+
 /*
   An abstract class for creating/running/joining threads.
   Thread::start() will create a new pthread, and execute the run() function.
@@ -28,7 +30,11 @@ namespace thread {
 class Thread
 {
 public:
-  Thread() : m_thread_id(0) {}
+  Thread() : m_thread_id(0)
+#ifdef __WIN__
+    , m_thread_handle(0)
+#endif
+  {}
   virtual ~Thread();
 
   /*
@@ -51,7 +57,7 @@ public:
     Users should *not* call this function directly, they should rather
     invoke the start() function.
   */
-  static void run_wrapper(Thread*);
+  static void run_wrapper(Thread_start_arg*);
 
 protected:
   /*
@@ -63,6 +69,10 @@ protected:
 
 private:
   pthread_t m_thread_id;
+#ifdef __WIN__
+  // We need an open handle to the thread in order to join() it.
+  HANDLE m_thread_handle;
+#endif
 
   Thread(const Thread&);                        /* Not copyable. */
   void operator=(const Thread&);                /* Not assignable. */
@@ -74,10 +84,10 @@ private:
 class Mutex_lock
 {
 public:
-  Mutex_lock(pthread_mutex_t *mutex);
+  Mutex_lock(mysql_mutex_t *mutex);
   ~Mutex_lock();
 private:
-  pthread_mutex_t *m_mutex;
+  mysql_mutex_t *m_mutex;
 
   Mutex_lock(const Mutex_lock&);                /* Not copyable. */
   void operator=(const Mutex_lock&);            /* Not assignable. */
@@ -96,8 +106,8 @@ public:
   void notify();
 private:
   bool            m_notified;
-  pthread_cond_t  m_cond;
-  pthread_mutex_t m_mutex;
+  mysql_cond_t    m_cond;
+  mysql_mutex_t   m_mutex;
 
   Notification(const Notification&);            /* Not copyable. */
   void operator=(const Notification&);          /* Not assignable. */


Attachment: [text/bzr-bundle] bzr/tor.didriksen@sun.com-20100722071424-vnflyslt1wrtovcl.bundle
Thread
bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3319) Bug#55434Tor Didriksen22 Jul