Re: [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

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

[Paul Gofman]

Hi Piotr,

   thank you for the review. I've updated the patch. I had to move
_endthread up in the code to make it available in
_beginthread_trampoline, so patch became longer.

Regards,
    Paul.