[PATCH 1/5] msvcrt: Make _beginthread() error out as documented.
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 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index c2fc863dd33..8bf8d9327c1 100644 --- a/dlls/msvcrt/thread.c +++ b/dlls/msvcrt/thread.c @@ -118,6 +118,8 @@ uintptr_t CDECL _beginthread( TRACE("(%p, %d, %p)\n", start_address, stack_size, arglist); + if (!MSVCRT_CHECK_PMT(start_address)) return -1; + trampoline = malloc(sizeof(*trampoline)); if(!trampoline) { *_errno() = EAGAIN; @@ -128,7 +130,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). Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 47 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 8bf8d9327c1..d342d57d38c 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; @@ -147,6 +150,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.@) */ @@ -158,12 +177,30 @@ 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); + /* FIXME: may use different errno / return values */ + if (!MSVCRT_CHECK_PMT(start_address)) return 0; + + if (!(trampoline = malloc(sizeof(*trampoline)))) + 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) { + free(trampoline); + msvcrt_set_errno(GetLastError()); + return 0; + } + + return (uintptr_t)thread; } #if _MSVCR_VER>=80 -- 2.31.1
Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>
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 669d8c29a98..ad366a74d86 100644 --- a/dlls/msvcrt/msvcrt.h +++ b/dlls/msvcrt/msvcrt.h @@ -168,6 +168,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 d342d57d38c..01500d93d91 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(); } @@ -163,6 +184,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
Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>
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
Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>
Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/ucrtbase/tests/thread.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/ucrtbase/tests/thread.c b/dlls/ucrtbase/tests/thread.c index 01f46029c06..512d2158b66 100644 --- a/dlls/ucrtbase/tests/thread.c +++ b/dlls/ucrtbase/tests/thread.c @@ -132,6 +132,39 @@ static void test_thread_library_reference(char *thread_dll, CloseHandle(detach_event); } +static BOOL handler_called; + +void CDECL test_invalid_parameter_handler(const wchar_t *expression, + const wchar_t *function_name, + const wchar_t *file_name, + unsigned line_number, + uintptr_t reserved) +{ + handler_called = TRUE; +} + +static void test_thread_invalid_params(void) +{ + uintptr_t hThread; + _invalid_parameter_handler old = _set_invalid_parameter_handler(test_invalid_parameter_handler); + + errno = 0; + handler_called = FALSE; + hThread = _beginthreadex(NULL, 0, NULL, NULL, 0, NULL); + ok(hThread == 0, "_beginthreadex unexpected ret: %d\n", hThread); + ok(errno == EINVAL, "_beginthreadex unexpected errno: %d\n", errno); + ok(handler_called, "Expected invalid_parameter_handler to be called\n"); + + errno = 0; + handler_called = FALSE; + hThread = _beginthread(NULL, 0, NULL); + ok(hThread == -1, "_beginthread unexpected ret: %d\n", hThread); + ok(errno == EINVAL, "_beginthread unexpected errno: %d\n", errno); + ok(handler_called, "Expected invalid_parameter_handler to be called\n"); + + _set_invalid_parameter_handler(old); +} + START_TEST(thread) { BOOL ret; @@ -144,4 +177,6 @@ START_TEST(thread) ret = DeleteFileA(thread_dll); ok(ret, "Failed to remove the test dll, err: %u", GetLastError()); + + test_thread_invalid_params(); } -- 2.31.1
Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>
participants (2)
-
Arkadiusz Hiler -
Piotr Caban