Paul Gofman : msvcrt: Handle of thread created by _beginthread should not be closed on thread detach.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: faf59e5f06a9ccf1bb6ae83716e68f63b883c001
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=faf59e5f06a9ccf1bb6ae83716...

Author: Paul Gofman <gofmanp(a)gmail.com>
Date:   Mon Feb  1 15:16:01 2016 +0300

msvcrt: Handle of thread created by _beginthread should not be closed on thread detach.

Handle should be closed if _endthread is called or thread function
exits. If thread is terminated by ExitThread or _endthreadex, thread
handle remains valid.

Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 dlls/msvcrt/main.c       |  1 -
 dlls/msvcrt/tests/misc.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 dlls/msvcrt/thread.c     | 56 +++++++++++++++++++++++++------------------
 3 files changed, 95 insertions(+), 24 deletions(-)

diff --git a/dlls/msvcrt/main.c b/dlls/msvcrt/main.c
index 6973dca..a930a32 100644
--- a/dlls/msvcrt/main.c
+++ b/dlls/msvcrt/main.c
@@ -67,7 +67,6 @@ static inline void msvcrt_free_tls_mem(void)
 
   if (tls)
   {
-    CloseHandle(tls->handle);
     MSVCRT_free(tls->efcvt_buffer);
     MSVCRT_free(tls->asctime_buffer);
     MSVCRT_free(tls->wasctime_buffer);
diff --git a/dlls/msvcrt/tests/misc.c b/dlls/msvcrt/tests/misc.c
index c568361..4968cbb 100644
--- a/dlls/msvcrt/tests/misc.c
+++ b/dlls/msvcrt/tests/misc.c
@@ -23,6 +23,7 @@
 #include <stdio.h>
 #include <math.h>
 #include "msvcrt.h"
+#include <process.h>
 
 static inline float __port_infinity(void)
 {
@@ -540,6 +541,66 @@ static void test_math_functions(void)
     ok(errno == 0xdeadbeef, "errno = %d\n", errno);
 }
 
+static void __cdecl test_thread_func(void *end_thread_type)
+{
+    if (end_thread_type == (void*)1)
+        _endthread();
+    else if (end_thread_type == (void*)2)
+        ExitThread(0);
+    else if (end_thread_type == (void*)3)
+        _endthreadex(0);
+}
+
+static unsigned __stdcall test_thread_func_ex(void *arg)
+{
+    _endthread();
+    return 0;
+}
+
+static void test_thread_handle_close(void)
+{
+    HANDLE hThread;
+    DWORD ret;
+
+    /* _beginthread: handle is not closed on ExitThread and _endthreadex */
+    hThread = (HANDLE)_beginthread(test_thread_func, 0, (void*)0);
+    ok(hThread != INVALID_HANDLE_VALUE, "_beginthread failed (%d)\n", errno);
+    WaitForSingleObject(hThread, INFINITE);
+    ret = CloseHandle(hThread);
+    ok(!ret, "ret = %d\n", ret);
+
+    hThread = (HANDLE)_beginthread(test_thread_func, 0, (void*)1);
+    ok(hThread != INVALID_HANDLE_VALUE, "_beginthread failed (%d)\n", errno);
+    WaitForSingleObject(hThread, INFINITE);
+    ret = CloseHandle(hThread);
+    ok(!ret, "ret = %d\n", ret);
+
+    hThread = (HANDLE)_beginthread(test_thread_func, 0, (void*)2);
+    ok(hThread != INVALID_HANDLE_VALUE, "_beginthread failed (%d)\n", errno);
+    Sleep(150);
+    ret = WaitForSingleObject(hThread, INFINITE);
+    ok(ret == WAIT_OBJECT_0, "ret = %d\n", ret);
+    ret = CloseHandle(hThread);
+    ok(ret, "ret = %d\n", ret);
+
+    hThread = (HANDLE)_beginthread(test_thread_func, 0, (void*)3);
+    ok(hThread != INVALID_HANDLE_VALUE, "_beginthread failed (%d)\n", errno);
+    Sleep(150);
+    ret = WaitForSingleObject(hThread, INFINITE);
+    ok(ret == WAIT_OBJECT_0, "ret = %d\n", ret);
+    ret = CloseHandle(hThread);
+    ok(ret, "ret = %d\n", ret);
+
+    /* _beginthreadex: handle is not closed on _endthread */
+    hThread = (HANDLE)_beginthreadex(NULL,0, test_thread_func_ex, NULL, 0, NULL);
+    ok(hThread != NULL, "_beginthreadex failed (%d)\n", errno);
+    Sleep(150);
+    ret = WaitForSingleObject(hThread, INFINITE);
+    ok(ret == WAIT_OBJECT_0, "ret = %d\n", ret);
+    ret = CloseHandle(hThread);
+    ok(ret, "ret = %d\n", ret);
+}
+
 START_TEST(misc)
 {
     int arg_c;
@@ -568,4 +629,5 @@ START_TEST(misc)
     test__invalid_parameter();
     test_qsort_s();
     test_math_functions();
+    test_thread_handle_close();
 }
diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c
index 5e62354..da1f8bf 100644
--- a/dlls/msvcrt/thread.c
+++ b/dlls/msvcrt/thread.c
@@ -55,6 +55,38 @@ thread_data_t *msvcrt_get_thread_data(void)
     return ptr;
 }
 
+/*********************************************************************
+ *		_endthread (MSVCRT.@)
+ */
+void CDECL _endthread(void)
+{
+  thread_data_t *tls;
+
+  TRACE("(void)\n");
+
+  tls = TlsGetValue(msvcrt_tls_index);
+  if (tls && tls->handle != INVALID_HANDLE_VALUE)
+  {
+      CloseHandle(tls->handle);
+      tls->handle = INVALID_HANDLE_VALUE;
+  } else
+      WARN("tls=%p tls->handle=%p\n", tls, tls ? tls->handle : INVALID_HANDLE_VALUE);
+
+  /* FIXME */
+  ExitThread(0);
+}
+
+/*********************************************************************
+ *		_endthreadex (MSVCRT.@)
+ */
+void CDECL _endthreadex(
+  unsigned int retval) /* [in] Thread exit code */
+{
+  TRACE("(%d)\n", retval);
+
+  /* FIXME */
+  ExitThread(retval);
+}
 
 /*********************************************************************
  *		_beginthread_trampoline
@@ -69,6 +101,7 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg)
     MSVCRT_free(arg);
 
     local_trampoline.start_address(local_trampoline.arglist);
+    _endthread();
     return 0;
 }
 
@@ -132,29 +165,6 @@ MSVCRT_uintptr_t CDECL _beginthreadex(
 }
 
 /*********************************************************************
- *		_endthread (MSVCRT.@)
- */
-void CDECL _endthread(void)
-{
-  TRACE("(void)\n");
-
-  /* FIXME */
-  ExitThread(0);
-}
-
-/*********************************************************************
- *		_endthreadex (MSVCRT.@)
- */
-void CDECL _endthreadex(
-  unsigned int retval) /* [in] Thread exit code */
-{
-  TRACE("(%d)\n", retval);
-
-  /* FIXME */
-  ExitThread(retval);
-}
-
-/*********************************************************************
  *		_getptd (MSVCR80.@)
  */
 thread_data_t* CDECL _getptd(void)