MySQL Bugs: #26564: Windows implementation of pthread

Bug #26564	Windows implementation of pthread_join crashes
Submitted:	22 Feb 2007 11:02	Modified:	3 Jul 2007 19:32
Reporter:	Paul McCullagh (Basic Quality Contributor) (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server	Severity:	S2 (Serious)
Version:	5.1	OS:	Windows (Windows)
Assigned to:	Georgi Kodinov	CPU Architecture:	Any
Tags:	CloseHandle, pthread_join, qc, windows

Description:
The Windows implementation of pthread_join crashes in CloseHandle:

#define pthread_join(A,B) \
  ((WaitForSingleObject((A), INFINITE) != WAIT_OBJECT_0) || !CloseHandle(A))

The reason is, the thread is created with _beginthread(). According to the Win API docs:

"_endthread automatically closes the thread handle (whereas _endthreadex does not). Therefore, when using _beginthread and _endthread, do not explicitly close the thread handle by calling the Win32 CloseHandle API."

How to repeat:
Create a small C program under Windows that creates a thread using pthread_create(), and then waits for it to exit with pthread_join().

Suggested fix:
Either remove the CloseHandle(), or use beginthreadex() instead of _beginthread().

test case

Attachment: bug26564.c (text/plain), 824 bytes.

Thank you for the report.

Verified as described.

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/29533

ChangeSet@1.2515, 2007-06-25 19:21:18+03:00, gkodinov@magare.gmz +1 -0
  Bug #26564: Windows implementation of pthread_join crashes
  
  MySQL uses _beginthread()/_endthread() instead of 
  _beginthreadex()/_endthreadex() to create/end its threads
  on Windows.
  According to MSDN  _endthread() does close the thread handle.
  So there's no need the handle to be closed explicitly.
  Besides : WaitForSingleObject(, INFINITE) != WAIT_OBJECT_0) is
  true for all practical cases as the other two possible return 
  codes (according to MSDN) cannot happen in that case the 
  CloseHandle() was actually a dead code.
  Fixed by removing the CloseHandle() call. No test case added
  because it's not possible to test for absence of dead code.

Pushed into 5.1.21-beta

Noted in 5.1.21 changelog.

The Windows implementation of pthread_join() was incorrect and could 
cause crashes.

It's actually very strange how the MySQL Server could crash from this since there is no call to 'pthread_join' anywhere in the server code, the only place I find is in instance manager and NDB.

The function is "implemented" on Windows, but the threads we create in MySQL Server are not joinable, we always use PTHREAD_CREATE_DETACHED when initializing the pthread_attr_t. Thus we can use  _beginthread to create them on Windows (if you use _beginthreadex, you must "join/WaitForSingleObject" to avoid getting a memory leak), with _beginthread you can't wait for it!

This was discovered as part of porting NDB to Windows and a fix making it possibel to create also joinable threads,  has been implemented that will appear in a future version of MySQL.

I just added a call to pthread_join() in mysqltest as a fix for Bug #55426. Now I see some tests timing out on Windows in 5.1-mtr, and this seems to be caused by pthread_join() hanging indefintely when the thread in question has already exited, at least if it's been some time since it did.

main.marge and innodb.innodb_misc1 are thus affected.

I'm tempted to just #ifndef away the call on Windows: the problem it solves has not been observed on Windows and the different implementation of pthread_join() in 5.5+ does not appear to show this problem.