[PATCH 1/7] msvcrt: Remove return after _endthread().
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 1 - 1 file changed, 1 deletion(-) diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 650afdc08af..c2fc863dd33 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -103,7 +103,6 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg) local_trampoline.start_address(local_trampoline.arglist); _endthread(); - return 0; } /********************************************************************* -- 2.31.1
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/tests/misc.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/msvcrt/tests/misc.c b/dlls/msvcrt/tests/misc.c index e262983e7e6..b38915596df 100644 --- a/dlls/msvcrt/tests/misc.c +++ b/dlls/msvcrt/tests/misc.c @@ -587,6 +587,19 @@ static void test_thread_handle_close(void) ok(ret, "ret = %d\n", ret); } +static void test_thread_suspended(void) +{ + HANDLE hThread; + DWORD ret; + + hThread = (HANDLE)_beginthreadex(NULL, 0, test_thread_func_ex, NULL, CREATE_SUSPENDED, NULL); + ok(hThread != NULL, "_beginthreadex failed (%d)\n", errno); + ret = ResumeThread(hThread); + ok(ret == 1, "suspend count = %d\n", ret); + ret = WaitForSingleObject(hThread, 200); + ok(ret == WAIT_OBJECT_0, "ret = %d\n", ret); +} + static int __cdecl _lfind_s_comp(void *ctx, const void *l, const void *r) { *(int *)ctx = 0xdeadc0de; @@ -684,5 +697,6 @@ START_TEST(misc) test_qsort_s(); test_math_functions(); test_thread_handle_close(); + test_thread_suspended(); test__lfind_s(); } -- 2.31.1
Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>
The functions uses EACCESS for insufficient resources, so we cannot leave errno set by malloc(). msvcrt_set_errno() seems to be doing the right thing in case of too many threads, invalid parameters, etc. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index c2fc863dd33..3ab53166192 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -118,9 +118,14 @@ uintptr_t CDECL _beginthread( TRACE("(%p, %d, %p)\n", start_address, stack_size, arglist); + if(!start_address) { + *_errno() = EINVAL; + return -1; + } + trampoline = malloc(sizeof(*trampoline)); if(!trampoline) { - *_errno() = EAGAIN; + *_errno() = EACCES; return -1; } @@ -128,7 +133,7 @@ uintptr_t CDECL _beginthread( trampoline, CREATE_SUSPENDED, NULL); if(!thread) { free(trampoline); - *_errno() = EAGAIN; + msvcrt_set_errno(GetLastError()); return -1; } -- 2.31.1
This way we can call _endthreadex() at the end as stated in the documentation. This will also simplify FreeLibrary()-safe implementation for UCRT. Thread handle in TLS is set to INVALID_HANDLE_VALUE, so that accidental _endthread() won't close it, see: existing tests. The function is documented to return 0 (unlike _beginthreadex() which returns -1) on errors other than start_routine == NULL. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 52 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 3ab53166192..9c0a5e414ad 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -27,7 +27,10 @@ WINE_DEFAULT_DEBUG_CHANNEL(msvcrt); typedef struct { HANDLE thread; - _beginthread_start_routine_t start_address; + union { + _beginthread_start_routine_t start_address; + _beginthreadex_start_routine_t start_address_ex; + }; void *arglist; } _beginthread_trampoline_t; @@ -150,6 +153,22 @@ uintptr_t CDECL _beginthread( return (uintptr_t)thread; } +/********************************************************************* + * _beginthreadex_trampoline + */ +static DWORD CALLBACK _beginthreadex_trampoline(LPVOID arg) +{ + unsigned int retval; + _beginthread_trampoline_t local_trampoline; + thread_data_t *data = msvcrt_get_thread_data(); + + memcpy(&local_trampoline, arg, sizeof(local_trampoline)); + data->handle = local_trampoline.thread; + free(arg); + + retval = local_trampoline.start_address_ex(local_trampoline.arglist); + _endthreadex(retval); +} /********************************************************************* * _beginthreadex (MSVCRT.@) */ @@ -161,12 +180,35 @@ uintptr_t CDECL _beginthreadex( unsigned int initflag, /* [in] Initial state of new thread (0 for running or CREATE_SUSPEND for suspended) */ unsigned int *thrdaddr) /* [out] Points to a 32-bit variable that receives the thread identifier */ { + _beginthread_trampoline_t* trampoline; + HANDLE thread; + TRACE("(%p, %d, %p, %p, %d, %p)\n", security, stack_size, start_address, arglist, initflag, thrdaddr); - /* FIXME */ - return (uintptr_t)CreateThread(security, stack_size, - start_address, arglist, - initflag, thrdaddr); + if(!start_address) { + *_errno() = EINVAL; + return -1; + } + + /* FIXME: may use different errno / return values */ + trampoline = malloc(sizeof(*trampoline)); + if(!trampoline) { + *_errno() = EACCES; + return 0; + } + + trampoline->thread = INVALID_HANDLE_VALUE; + trampoline->start_address_ex = start_address; + trampoline->arglist = arglist; + + thread = CreateThread(security, stack_size, _beginthreadex_trampoline, + trampoline, initflag, thrdaddr); + if(!thread) { + msvcrt_set_errno(GetLastError()); + return 0; + } + + return (uintptr_t)thread; } #if _MSVCR_VER>=80 -- 2.31.1
Hi Arek, On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
+/********************************************************************* + * _beginthreadex_trampoline + */ +static DWORD CALLBACK _beginthreadex_trampoline(LPVOID arg) +{ + unsigned int retval; + _beginthread_trampoline_t local_trampoline; + thread_data_t *data = msvcrt_get_thread_data(); + + memcpy(&local_trampoline, arg, sizeof(local_trampoline)); + data->handle = local_trampoline.thread; + free(arg); + + retval = local_trampoline.start_address_ex(local_trampoline.arglist); + _endthreadex(retval); +} Compilation warning: warning: no return statement in function returning non-void
/********************************************************************* * _beginthreadex (MSVCRT.@) */ @@ -161,12 +180,35 @@ uintptr_t CDECL _beginthreadex( unsigned int initflag, /* [in] Initial state of new thread (0 for running or CREATE_SUSPEND for suspended) */ unsigned int *thrdaddr) /* [out] Points to a 32-bit variable that receives the thread identifier */ { + _beginthread_trampoline_t* trampoline; + HANDLE thread; + TRACE("(%p, %d, %p, %p, %d, %p)\n", security, stack_size, start_address, arglist, initflag, thrdaddr);
- /* FIXME */ - return (uintptr_t)CreateThread(security, stack_size, - start_address, arglist, - initflag, thrdaddr); + if(!start_address) { + *_errno() = EINVAL; + return -1; + } The function returns 0 in ucrtbase on invalid argument. I didn't test what version it changes in.
Thanks, Piotr
On Tue, May 04, 2021 at 02:46:24PM +0200, Piotr Caban wrote:
Hi Arek,
On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
+/********************************************************************* + * _beginthreadex_trampoline + */ +static DWORD CALLBACK _beginthreadex_trampoline(LPVOID arg) +{ + unsigned int retval; + _beginthread_trampoline_t local_trampoline; + thread_data_t *data = msvcrt_get_thread_data(); + + memcpy(&local_trampoline, arg, sizeof(local_trampoline)); + data->handle = local_trampoline.thread; + free(arg); + + retval = local_trampoline.start_address_ex(local_trampoline.arglist); + _endthreadex(retval); +} Compilation warning: warning: no return statement in function returning non-void
I'll add the retrun then. What are you compiling those with? I don't get those warnings locally due to DECLSPEC_NORETURN on _endthread[ex](). Marvin also doesn't seem to complain.
/********************************************************************* * _beginthreadex (MSVCRT.@) */ @@ -161,12 +180,35 @@ uintptr_t CDECL _beginthreadex( unsigned int initflag, /* [in] Initial state of new thread (0 for running or CREATE_SUSPEND for suspended) */ unsigned int *thrdaddr) /* [out] Points to a 32-bit variable that receives the thread identifier */ { + _beginthread_trampoline_t* trampoline; + HANDLE thread; + TRACE("(%p, %d, %p, %p, %d, %p)\n", security, stack_size, start_address, arglist, initflag, thrdaddr); - /* FIXME */ - return (uintptr_t)CreateThread(security, stack_size, - start_address, arglist, - initflag, thrdaddr); + if(!start_address) { + *_errno() = EINVAL; + return -1; + } The function returns 0 in ucrtbase on invalid argument. I didn't test what version it changes in.
Oh, it's even more horrible than what's documented... Thanks for spotting this. I'll do a sweep and add tests in ucrtbase. I wonder if the other returns are consistent wrt returning 0 and -1. Sadly, I haven't manage to make beginthread fail in any other way while testing. -- Cheers, Arek
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/tests/misc.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/dlls/msvcrt/tests/misc.c b/dlls/msvcrt/tests/misc.c index b38915596df..b8d6e24dbd0 100644 --- a/dlls/msvcrt/tests/misc.c +++ b/dlls/msvcrt/tests/misc.c @@ -600,6 +600,21 @@ static void test_thread_suspended(void) ok(ret == WAIT_OBJECT_0, "ret = %d\n", ret); } +static void test_thread_invalid_params(void) +{ + uintptr_t hThread; + + errno = 0; + hThread = _beginthreadex(NULL, 0, NULL, NULL, 0, NULL); + ok(hThread == -1, "_beginthreadex unexpected ret: %d\n", hThread); + ok(errno == EINVAL, "_beginthreadex unexpected errno: %d\n", errno); + + errno = 0; + hThread = _beginthread(NULL, 0, NULL); + ok(hThread == -1, "_beginthread unexpected ret: %d\n", hThread); + ok(errno == EINVAL, "_beginthread unexpected errno: %d\n", errno); +} + static int __cdecl _lfind_s_comp(void *ctx, const void *l, const void *r) { *(int *)ctx = 0xdeadc0de; @@ -698,5 +713,6 @@ START_TEST(misc) test_math_functions(); test_thread_handle_close(); test_thread_suspended(); + test_thread_invalid_params(); test__lfind_s(); } -- 2.31.1
MSVCRT's _beginthread[ex]() doesn't exhibit the same behavior and using ThreadExit() does leak the reference. FreeLibraryAndExit() has to be used because the DLL may be the only user of the given CRT. This fixes Baldur's Gate 3 crashing shortly after launch. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/msvcrt.h | 1 + dlls/msvcrt/thread.c | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h index 8f6ee08ef2a..c84e0a0a638 100644 --- a/dlls/msvcrt/msvcrt.h +++ b/dlls/msvcrt/msvcrt.h @@ -167,6 +167,7 @@ struct __thread_data { void *unk10[100]; #if _MSVCR_VER >= 140 _invalid_parameter_handler invalid_parameter_handler; + HMODULE module; #endif }; diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 9c0a5e414ad..c106548adc1 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -54,6 +54,9 @@ thread_data_t *CDECL msvcrt_get_thread_data(void) ptr->random_seed = 1; ptr->locinfo = MSVCRT_locale->locinfo; ptr->mbcinfo = MSVCRT_locale->mbcinfo; +#if _MSVCR_VER >= 140 + ptr->module = NULL; +#endif } SetLastError( err ); return ptr; @@ -76,8 +79,7 @@ void CDECL _endthread(void) } else WARN("tls=%p tls->handle=%p\n", tls, tls ? tls->handle : INVALID_HANDLE_VALUE); - /* FIXME */ - ExitThread(0); + _endthreadex(0); } /********************************************************************* @@ -88,7 +90,17 @@ void CDECL _endthreadex( { TRACE("(%d)\n", retval); - /* FIXME */ +#if _MSVCR_VER >= 140 + { + thread_data_t *tls = TlsGetValue(msvcrt_tls_index); + + if (tls && tls->module != NULL) + FreeLibraryAndExitThread(tls->module, retval); + else + WARN("tls=%p tls->module=%p\n", tls, tls ? tls->module : NULL); + } +#endif + ExitThread(retval); } @@ -104,6 +116,15 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg) data->handle = local_trampoline.thread; free(arg); +#if _MSVCR_VER >= 140 + if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, + (void*)local_trampoline.start_address, &data->module)) + { + data->module = NULL; + WARN("failed to get module for the start_address: %d\n", GetLastError()); + } +#endif + local_trampoline.start_address(local_trampoline.arglist); _endthread(); } @@ -166,6 +187,18 @@ static DWORD CALLBACK _beginthreadex_trampoline(LPVOID arg) data->handle = local_trampoline.thread; free(arg); +#if _MSVCR_VER >= 140 + { + thread_data_t *data = msvcrt_get_thread_data(); + if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, + (void*)local_trampoline.start_address_ex, &data->module)) + { + data->module = NULL; + WARN("failed to get module for the start_address: %d\n", GetLastError()); + } + } +#endif + retval = local_trampoline.start_address_ex(local_trampoline.arglist); _endthreadex(retval); } -- 2.31.1
Hi Arek, On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
/********************************************************************* @@ -88,7 +90,17 @@ void CDECL _endthreadex( { TRACE("(%d)\n", retval);
- /* FIXME */ +#if _MSVCR_VER >= 140 + { + thread_data_t *tls = TlsGetValue(msvcrt_tls_index); + + if (tls && tls->module != NULL) + FreeLibraryAndExitThread(tls->module, retval); + else + WARN("tls=%p tls->module=%p\n", tls, tls ? tls->module : NULL); + } +#endif + ExitThread(retval); } I didn't test it but I bet that _endthreadex should still close the handle if thread was created by _beginthread. It will also make the _endthread and _endthreadex functions identical. Did you check it?
Thanks, Piotr
On Tue, May 04, 2021 at 02:39:17PM +0200, Piotr Caban wrote:
Hi Arek,
On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
/********************************************************************* @@ -88,7 +90,17 @@ void CDECL _endthreadex( { TRACE("(%d)\n", retval); - /* FIXME */ +#if _MSVCR_VER >= 140 + { + thread_data_t *tls = TlsGetValue(msvcrt_tls_index); + + if (tls && tls->module != NULL) + FreeLibraryAndExitThread(tls->module, retval); + else + WARN("tls=%p tls->module=%p\n", tls, tls ? tls->module : NULL); + } +#endif + ExitThread(retval); } I didn't test it but I bet that _endthreadex should still close the handle if thread was created by _beginthread. It will also make the _endthread and _endthreadex functions identical. Did you check it?
I belive this is covered by msvcrt/tests/misc.c:test_thread_handle_close(): https://source.winehq.org/git/wine.git/blob/3ba4412be60dafee310b5d3c71aa762a... -- Cheers, Arek
On 5/4/21 5:26 PM, Arkadiusz Hiler wrote:
On Tue, May 04, 2021 at 02:39:17PM +0200, Piotr Caban wrote:
Hi Arek,
On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
/********************************************************************* @@ -88,7 +90,17 @@ void CDECL _endthreadex( { TRACE("(%d)\n", retval); - /* FIXME */ +#if _MSVCR_VER >= 140 + { + thread_data_t *tls = TlsGetValue(msvcrt_tls_index); + + if (tls && tls->module != NULL) + FreeLibraryAndExitThread(tls->module, retval); + else + WARN("tls=%p tls->module=%p\n", tls, tls ? tls->module : NULL); + } +#endif + ExitThread(retval); } I didn't test it but I bet that _endthreadex should still close the handle if thread was created by _beginthread. It will also make the _endthread and _endthreadex functions identical. Did you check it?
I belive this is covered by msvcrt/tests/misc.c:test_thread_handle_close():
https://source.winehq.org/git/wine.git/blob/3ba4412be60dafee310b5d3c71aa762a... Yes, you're right. It's another difference between msvcrt and ucrtbase. The handle is closed in endthreadex if you run the tests with ucrtbase.
Thanks, Piotr
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/ucrtbase/tests/Makefile.in | 5 +- dlls/ucrtbase/tests/thread.c | 147 +++++++++++++++++++++++++++++ dlls/ucrtbase/tests/threaddll.c | 67 +++++++++++++ dlls/ucrtbase/tests/threaddll.h | 30 ++++++ dlls/ucrtbase/tests/threaddll.spec | 3 + 5 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 dlls/ucrtbase/tests/thread.c create mode 100644 dlls/ucrtbase/tests/threaddll.c create mode 100644 dlls/ucrtbase/tests/threaddll.h create mode 100644 dlls/ucrtbase/tests/threaddll.spec diff --git a/dlls/ucrtbase/tests/Makefile.in b/dlls/ucrtbase/tests/Makefile.in index 259880a7d90..7146d4d1f02 100644 --- a/dlls/ucrtbase/tests/Makefile.in +++ b/dlls/ucrtbase/tests/Makefile.in @@ -7,4 +7,7 @@ C_SRCS = \ misc.c \ printf.c \ scanf.c \ - string.c + string.c \ + thread.c \ + threaddll.c \ + threaddll.spec diff --git a/dlls/ucrtbase/tests/thread.c b/dlls/ucrtbase/tests/thread.c new file mode 100644 index 00000000000..01f46029c06 --- /dev/null +++ b/dlls/ucrtbase/tests/thread.c @@ -0,0 +1,147 @@ +/* + * Copyright 2021 Arkadiusz Hiler for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <errno.h> +#include <stdarg.h> +#include <process.h> + +#include <windef.h> +#include <winbase.h> +#include "wine/test.h" + +#include "threaddll.h" + +enum beginthread_method +{ + use_beginthread, + use_beginthreadex +}; + +static char *get_thread_dll_path(void) +{ + static char path[MAX_PATH]; + const char dll_name[] = "threaddll.dll"; + DWORD written; + HANDLE file; + HRSRC res; + void *ptr; + + GetTempPathA(ARRAY_SIZE(path), path); + strcat(path, dll_name); + + file = CreateFileA(path, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); + ok(file != INVALID_HANDLE_VALUE, "Failed to create file %s: %u.\n", + debugstr_a(path), GetLastError()); + + res = FindResourceA(NULL, dll_name, "TESTDLL"); + ok(!!res, "Failed to load resource: %u\n", GetLastError()); + ptr = LockResource(LoadResource(GetModuleHandleA(NULL), res)); + WriteFile(file, ptr, SizeofResource( GetModuleHandleA(NULL), res), &written, NULL); + ok(written == SizeofResource(GetModuleHandleA(NULL), res), "Failed to write resource\n"); + CloseHandle(file); + + return path; +} + +static void set_thead_dll_detach_event(HANDLE dll, HANDLE event) +{ + void WINAPI (*_set_detach_event)(HANDLE event); + _set_detach_event = (void*) GetProcAddress(dll, "set_detach_event"); + ok(_set_detach_event != NULL, "Failed to get set_detach_event: %u\n", GetLastError()); + _set_detach_event(event); +} + +static void test_thread_library_reference(char *thread_dll, + enum beginthread_method beginthread_method, + enum thread_exit_method exit_method) +{ + HANDLE detach_event; + HMODULE dll; + DWORD ret; + uintptr_t thread_handle; + struct threaddll_args args; + + args.exit_method = exit_method; + + detach_event = CreateEventA(NULL, FALSE, FALSE, NULL); + ok(detach_event != NULL, "Failed to create an event: %u\n", GetLastError()); + args.confirm_running = CreateEventA(NULL, FALSE, FALSE, NULL); + ok(args.confirm_running != NULL, "Failed to create an event: %u\n", GetLastError()); + args.past_free = CreateEventA(NULL, FALSE, FALSE, NULL); + ok(args.past_free != NULL, "Failed to create an event: %u\n", GetLastError()); + + dll = LoadLibraryA(thread_dll); + ok(dll != NULL, "Failed to load the test dll: %u\n", GetLastError()); + + set_thead_dll_detach_event(dll, detach_event); + + if (beginthread_method == use_beginthreadex) + { + _beginthreadex_start_routine_t proc = (void*) GetProcAddress(dll, "stdcall_thread_proc"); + ok(proc != NULL, "Failed to get stdcall_thread_proc: %u\n", GetLastError()); + thread_handle = _beginthreadex(NULL, 0, proc, &args, 0, NULL); + } + else + { + _beginthread_start_routine_t proc = (void*) GetProcAddress(dll, "cdecl_thread_proc"); + ok(proc != NULL, "Failed to get stdcall_thread_proc: %u\n", GetLastError()); + thread_handle = _beginthread(proc, 0, &args); + } + + ok(thread_handle != -1 && thread_handle != 0, "Failed to begin thread: %u\n", errno); + + ret = WaitForSingleObject(args.confirm_running, 200); + ok(ret == WAIT_OBJECT_0, "Event was not signaled, ret: %u, err: %u\n", ret, GetLastError()); + + ok(FreeLibrary(dll), "Failed to free the library: %u\n", GetLastError()); + + ret = WaitForSingleObject(detach_event, 0); + ok(ret == WAIT_TIMEOUT, "Thread detach happened unexpectedly signaling an event, ret: %d, err: %u\n", ret, GetLastError()); + + ok(SetEvent(args.past_free), "Failed to signal event: %d\n", GetLastError()); + + if (beginthread_method == use_beginthreadex) + { + ret = WaitForSingleObject((HANDLE)thread_handle, 200); + ok(ret == WAIT_OBJECT_0, "Thread has not exited, ret: %d, err: %u\n", ret, GetLastError()); + } + + ret = WaitForSingleObject(detach_event, 200); + ok(ret == WAIT_OBJECT_0, "Detach event was not signaled, ret: %d, err: %u\n", ret, GetLastError()); + + if (beginthread_method == use_beginthreadex) + CloseHandle((HANDLE)thread_handle); + + CloseHandle(args.past_free); + CloseHandle(args.confirm_running); + CloseHandle(detach_event); +} + +START_TEST(thread) +{ + BOOL ret; + char *thread_dll = get_thread_dll_path(); + + test_thread_library_reference(thread_dll, use_beginthread, thread_exit_return); + test_thread_library_reference(thread_dll, use_beginthread, thread_exit_endthread); + test_thread_library_reference(thread_dll, use_beginthreadex, thread_exit_return); + test_thread_library_reference(thread_dll, use_beginthreadex, thread_exit_endthreadex); + + ret = DeleteFileA(thread_dll); + ok(ret, "Failed to remove the test dll, err: %u", GetLastError()); +} diff --git a/dlls/ucrtbase/tests/threaddll.c b/dlls/ucrtbase/tests/threaddll.c new file mode 100644 index 00000000000..d11f95b91b0 --- /dev/null +++ b/dlls/ucrtbase/tests/threaddll.c @@ -0,0 +1,67 @@ +/* + * Copyright 2021 Arkadiusz Hiler for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <windows.h> + +#include "threaddll.h" + +static HANDLE detach_event; + +void CDECL _endthread(void); +void CDECL _endthreadex(unsigned int); + +void WINAPI set_detach_event(HANDLE event) +{ + detach_event = event; +} + +static unsigned internal_thread_proc(void *param) +{ + struct threaddll_args *args = param; + SetEvent(args->confirm_running); + WaitForSingleObject(args->past_free, INFINITE); + + if (args->exit_method == thread_exit_endthread) + _endthread(); + else if (args->exit_method == thread_exit_endthreadex) + _endthreadex(0); + + return 0; +} + +unsigned WINAPI stdcall_thread_proc(void *param) +{ + return internal_thread_proc(param); +} + +void CDECL cdecl_thread_proc(void *param) +{ + internal_thread_proc(param); +} + +BOOL WINAPI DllMain(HINSTANCE instance_new, DWORD reason, LPVOID reserved) +{ + switch (reason) + { + case DLL_PROCESS_DETACH: + if (detach_event) SetEvent(detach_event); + break; + } + + return TRUE; +} diff --git a/dlls/ucrtbase/tests/threaddll.h b/dlls/ucrtbase/tests/threaddll.h new file mode 100644 index 00000000000..1d8d085798f --- /dev/null +++ b/dlls/ucrtbase/tests/threaddll.h @@ -0,0 +1,30 @@ +/* + * Copyright 2021 Arkadiusz Hiler for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +enum thread_exit_method { + thread_exit_return, + thread_exit_endthread, + thread_exit_endthreadex +}; + +struct threaddll_args +{ + HANDLE confirm_running; + HANDLE past_free; + enum thread_exit_method exit_method; +}; diff --git a/dlls/ucrtbase/tests/threaddll.spec b/dlls/ucrtbase/tests/threaddll.spec new file mode 100644 index 00000000000..8422c096946 --- /dev/null +++ b/dlls/ucrtbase/tests/threaddll.spec @@ -0,0 +1,3 @@ +@ stdcall set_detach_event(ptr) +@ stdcall stdcall_thread_proc(ptr) +@ cdecl cdecl_thread_proc(ptr) -- 2.31.1
Hi Arek, On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 650afdc08af..c2fc863dd33 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -103,7 +103,6 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg)
local_trampoline.start_address(local_trampoline.arglist); _endthread(); - return 0; } Even so the return will be never reached removing it will cause a compiler warning.
Thanks, Piotr
On Tue, May 04, 2021 at 02:30:56PM +0200, Piotr Caban wrote:
Hi Arek,
On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 650afdc08af..c2fc863dd33 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -103,7 +103,6 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg) local_trampoline.start_address(local_trampoline.arglist); _endthread(); - return 0; } Even so the return will be never reached removing it will cause a compiler warning.
It should not create a warning as both _endthread and _endthreadex are DECLSPEC_NORETURN. There also seem to be a lot of places where we ExitThread() without having it followed by a return in non-void functions, so I don't think this is a complete novelty. Anyway, we can skip this patch if having a return here is preferred. It was just bugging me visually. -- Cheers, Arek
On 5/4/21 2:41 PM, Arkadiusz Hiler wrote:
On Tue, May 04, 2021 at 02:30:56PM +0200, Piotr Caban wrote:
Hi Arek,
On 5/4/21 1:49 PM, Arkadiusz Hiler wrote:
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 650afdc08af..c2fc863dd33 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -103,7 +103,6 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg) local_trampoline.start_address(local_trampoline.arglist); _endthread(); - return 0; } Even so the return will be never reached removing it will cause a compiler warning.
It should not create a warning as both _endthread and _endthreadex are DECLSPEC_NORETURN. There also seem to be a lot of places where we ExitThread() without having it followed by a return in non-void functions, so I don't think this is a complete novelty.
Anyway, we can skip this patch if having a return here is preferred. It was just bugging me visually.
Sorry, I was in middle of something and the warning was from other unfinished work. Taking in account it's already done this way in other places I think it's ok to remove the return statement.
participants (3)
-
Arkadiusz Hiler -
Piotr Caban -
Piotr Caban