#At file:///export/home/didrik/repo/next-mr-bf-bug55434/ based on revid:jonathan.perkin@stripped
3319 Tor Didriksen 2010-07-23
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-23 07:33:41 +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;
}
@@ -38,56 +65,93 @@ void assert_false(int arg, int line)
Thread::~Thread()
{
+#ifdef __WIN__
+ if (m_thread_handle != 0)
+ CloseHandle(m_thread_handle);
+#endif
}
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
+ ;
+ }
+#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 +165,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 +174,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-23 07:33:41 +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-20100723073341-rmv81iefgjhmkuzm.bundle