MySQL Bugs: #55434: multithreaded gunit tests fail on windows

Bug #55434	multithreaded gunit tests fail on windows
Submitted:	21 Jul 2010 10:56	Modified:	4 Aug 2010 19:57
Reporter:	Tor Didriksen	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Tests	Severity:	S3 (Non-critical)
Version:	5.6.99-M4	OS:	Any
Assigned to:	Tor Didriksen	CPU Architecture:	Any

Description:
Multithreaded gunit tests fail most of the time on windows.

This is because pthread_create() closes the handle of the thread.

Our version of pthread_join calls OpenThread to get a handle,
in order to do WaitForSingleObject().
This often fails with GetLastError=87 ERROR_INVALID_PARAMETER
which makes sense if the thread to join has already finished.

How to repeat:
run thread_utils-t or mdl-t on windows.

Suggested fix:
Keep an open handle to the thread, so that we can do WaitForSingleObject() deterministically.

The server never does pthread_join() on windows,
so the easier fix is to handle this in gunit/thread_utils.cc

I thnk it makes sense to always return 0 from pthread_join when encountering invalid parameter threadID.
OTOH, the pthread_join documentation on http://opengroup.org/onlinepubs/007908799/xsh/pthread_join.html

states that EINVAL needs to be returned when thread parameter does not refer to a joinable thread, which I'd think is exactly the case here (that joinable thread has already gone). Perhaps, ESRCH would be a better return code, if it has existed on Windows (it does not).

Alternative solution of keeping thread handles open is a whole lot more problematic, from my experience. MySQL codebase already had this, and it was very  hard to maintain spaghetti bowl. One will need to take are of closing handles, otherwise we'll have resource leaks. One needs  pthread_t defined as HANDLE, and this turns out problematic for pthread_self/pthread_equal - it cannot be solved as trivially as comparing integers. The old implementation is still active in 5.0. I reworked this stuff in 5.1, to use thread ids everywhere, it simplified things "alot".

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/114112

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

Looks ok,  however Thread will leak handle, if it is not joined.

You can fix it e.g like this:
- Initialize m_thread_handle to 0 in Thread constructor
- move CloseHandle(m_thread_handle) into Thread destructor (after checking for != 0)
- optionally, you also can close handle like now in Thread::join(), but you will also need to reset m_thread_handle to 0 to prevent destructor from closing invalid handle.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/114118

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

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/114206

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

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/114222

3321 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

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/114224

3322 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

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/114228

3323 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

Pushed to next-mr-bugfixing.

Pushed into mysql-trunk 5.6.1-m4 (revid:alik@ibmvm-20100804080001-bny5271e65xo34ig) (version source revid:marko.makela@oracle.com-20100621094008-o9fa153s3f09merw) (merge vers: 5.1.49) (pib:18)

Pushed into mysql-trunk 5.6.1-m4 (revid:alik@ibmvm-20100804081533-c1d3rbipo9e8rt1s) (version source revid:marko.makela@oracle.com-20100621094008-o9fa153s3f09merw) (merge vers: 5.1.49) (pib:18)

Pushed into mysql-next-mr (revid:alik@ibmvm-20100804081630-ntapn8bf9pko9vj3) (version source revid:marko.makela@oracle.com-20100621094008-o9fa153s3f09merw) (pib:20)

Changes to test suite. No changelog entry needed.