[v2] msvcrt: handle of thread created by _beginthread should not be closed on thread detach.

[Piotr Caban]

Hi Paul,

On 01/31/16 22:58, Paul Gofman wrote:
> +    /* _beginthread: handle is not closed on ExitProcess and _endthreadex */
> +    hThread = (HANDLE)_beginthread(test_thread_func, 0, (void*)0);
> +    Sleep(150);
> +    ret = WaitForSingleObject(hThread, INFINITE);
> +    ok(ret == WAIT_FAILED, "ret = %d\n", ret);
This test may fail if for some reason thread has not finished during 
150ms sleep. You can workaround this issue following way:
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);

> @@ -69,6 +69,9 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg)
>       MSVCRT_free(arg);
>
>       local_trampoline.start_address(local_trampoline.arglist);
> +
> +    CloseHandle(data->handle);
> +    data->handle = NULL;
According to MSDN _endthread should be called here. Because it's 
documented I think we should implement it in similar way.

> @@ -136,8 +139,18 @@ MSVCRT_uintptr_t CDECL _beginthreadex(
>    */
>   void CDECL _endthread(void)
>   {
> +  thread_data_t *tls;
> +
>     TRACE("(void)\n");
>
> +  tls = TlsGetValue(msvcrt_tls_index);
> +  if (tls && tls->handle)
You should check for INVALID_HANDLE_VALUE instead of NULL here. It's 
doesn't really matter but for sake of consistency tls->handle should be 
set to INVALID_HANDLE_VALUE after close.

Thanks,
Piotr