[PATCH 1/4] msvcrt: Add CREATE_SUSPENDED test for _beginthreadex().
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
This way we can call _endthreadex() at the end as stated in the documentation. This will also simplify FreeLibrary()-safe implementation for UCRT. thread_data->handle is not set on purpose, so that accidental _endthread() won't close it, see: existing tests. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/thread.c | 55 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/dlls/msvcrt/thread.c b/dlls/msvcrt/thread.c index 650afdc08af..523e463d695 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; @@ -146,6 +149,21 @@ 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; + + memcpy(&local_trampoline,arg,sizeof(local_trampoline)); + free(arg); + + retval = local_trampoline.start_address_ex(local_trampoline.arglist); + + _endthreadex(retval); +} /********************************************************************* * _beginthreadex (MSVCRT.@) */ @@ -157,12 +175,39 @@ 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); + trampoline = malloc(sizeof(*trampoline)); + if(!trampoline) { + *_errno() = EAGAIN; + return -1; + } + + thread = CreateThread(security, stack_size, _beginthreadex_trampoline, + trampoline, initflag | CREATE_SUSPENDED, thrdaddr); + if(!thread) { + free(trampoline); + *_errno() = EAGAIN; + return -1; + } + + trampoline->thread = thread; + trampoline->start_address_ex = start_address; + trampoline->arglist = arglist; + + if(initflag & CREATE_SUSPENDED) + return (uintptr_t)thread; + + if(ResumeThread(thread) == -1) { + free(trampoline); + *_errno() = EAGAIN; + return -1; + } + + return (uintptr_t)thread; } #if _MSVCR_VER>=80 -- 2.31.1
Arkadiusz Hiler <ahiler(a)codeweavers.com> wrote:
+ thread = CreateThread(security, stack_size, _beginthreadex_trampoline, + trampoline, initflag | CREATE_SUSPENDED, thrdaddr);
What's the reason of unconditionally creating a suspended thread? -- Dmitry.
On Mon, May 03, 2021 at 04:24:40PM +0300, Dmitry Timoshkov wrote:
Arkadiusz Hiler <ahiler(a)codeweavers.com> wrote:
+ thread = CreateThread(security, stack_size, _beginthreadex_trampoline, + trampoline, initflag | CREATE_SUSPENDED, thrdaddr);
What's the reason of unconditionally creating a suspended thread?
It's a leftover from initial copy-and-paste of the _beginthread()'s trampoline logic. The thread there is suspended because _endthread() closes the handle, and we only get that after creation. Here I was doing something simillar initially, but it turned out that even _endthread() should not close anything. I've changed a few things but forgot to change this part. Revised version of the patch is attached, thanks. -- Cheers, Arek
Hi Arek, On 5/3/21 3:44 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)); Please add spaces between arguments.
/********************************************************************* * _beginthreadex (MSVCRT.@) */ @@ -157,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); + trampoline = malloc(sizeof(*trampoline)); + if(!trampoline) { + *_errno() = EAGAIN; + return -1; malloc already sets errno. Resetting it to EAGAIN looks questionable. The function should also return 0 on error.
+ 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); + *_errno() = EAGAIN; + return -1; It should probably do something like: msvcrt_set_errno(GetLastError()); return 0;
It would be nice to do similar changes in _beginthread() as well. Thanks, Piotr
On Mon, May 03, 2021 at 05:44:59PM +0200, Piotr Caban wrote:
Hi Arek,
On 5/3/21 3:44 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)); Please add spaces between arguments.
/********************************************************************* * _beginthreadex (MSVCRT.@) */ @@ -157,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); + trampoline = malloc(sizeof(*trampoline)); + if(!trampoline) { + *_errno() = EAGAIN; + return -1; malloc already sets errno. Resetting it to EAGAIN looks questionable. The function should also return 0 on error.
+ 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); + *_errno() = EAGAIN; + return -1; It should probably do something like: msvcrt_set_errno(GetLastError()); return 0;
The return -1 here is my bad, because I've misread MSDN, but looks like _beginthread() has a different behavior. _beginthreadex() returns zero and is mentioned to set errno and _doserrno (I'll include that in next revision) whithout any details.
It would be nice to do similar changes in _beginthread() as well.
_beginthread() is documented to: * return -1L * set errno to EAGAIN in too many threads * EINVAL for invalid argument / stack size is incorrect * EACCESS if insufficient resources (e.g. memory) I can clean it up a bit an make it more in line with what the spec says. -- Cheers, Arek
MSVCRT _beginthread[ex]() doesn't exhibit the same behavior. Using ThreadExit() does leak the reference. FreeLibraryAndExit() has to be used because the DLL may be the only user of the given CRT. Signed-off-by: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/msvcrt/msvcrt.h | 1 + dlls/msvcrt/thread.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 2 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 523e463d695..b796f7ac852 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,7 +79,13 @@ void CDECL _endthread(void) } else WARN("tls=%p tls->handle=%p\n", tls, tls ? tls->handle : INVALID_HANDLE_VALUE); - /* FIXME */ +#if _MSVCR_VER >= 140 + if (tls && tls->module != NULL) + FreeLibraryAndExitThread(tls->module, 0); + else + WARN("tls=%p tls->module=%p\n", tls, tls ? tls->module : NULL); +#endif + ExitThread(0); } @@ -88,7 +97,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 +123,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(); return 0; @@ -160,6 +188,18 @@ static DWORD CALLBACK _beginthreadex_trampoline(LPVOID arg) memcpy(&local_trampoline,arg,sizeof(local_trampoline)); 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
On 5/3/21 3:07 PM, Arkadiusz Hiler wrote:
@@ -76,7 +79,13 @@ void CDECL _endthread(void) } else WARN("tls=%p tls->handle=%p\n", tls, tls ? tls->handle : INVALID_HANDLE_VALUE);
- /* FIXME */ +#if _MSVCR_VER >= 140 + if (tls && tls->module != NULL) + FreeLibraryAndExitThread(tls->module, 0); + else + WARN("tls=%p tls->module=%p\n", tls, tls ? tls->module : NULL); +#endif + ExitThread(0); } Is there a reason for not calling _endthreadex(0) in _endthread?
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..ea76d149946 --- /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_beginthread) + { + 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 (exit_method != thread_exit_endthread) + 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..27b672eebc2 --- /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, 100); + + 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
On 5/3/21 3:07 PM, Arkadiusz Hiler wrote:
+static void test_thread_library_reference(char *thread_dll, + enum beginthread_method beginthread_method, + enum thread_exit_method exit_method) ... + if (exit_method != thread_exit_endthread) + CloseHandle((HANDLE)thread_handle); Shouldn't the CloseHandle be called depending on what function was called to create the thread?
+static unsigned internal_thread_proc(void *param) +{ + struct threaddll_args *args = param; + SetEvent(args->confirm_running); + WaitForSingleObject(args->past_free, 100); It doesn't matter that much in this case but please test WaitForSingleObject return value. It can still give some hints if tests start to fail.
On Mon, May 03, 2021 at 06:06:35PM +0200, Piotr Caban wrote:
On 5/3/21 3:07 PM, Arkadiusz Hiler wrote:
+static void test_thread_library_reference(char *thread_dll, + enum beginthread_method beginthread_method, + enum thread_exit_method exit_method) ... + if (exit_method != thread_exit_endthread) + CloseHandle((HANDLE)thread_handle); Shouldn't the CloseHandle be called depending on what function was called to create the thread?
Whoops, that's a leftover from when I thought that it depends purely on _endthread variant used.
+static unsigned internal_thread_proc(void *param) +{ + struct threaddll_args *args = param; + SetEvent(args->confirm_running); + WaitForSingleObject(args->past_free, 100); It doesn't matter that much in this case but please test WaitForSingleObject return value. It can still give some hints if tests start to fail.
Any standard way to do that from DLLs? The current winetest_*() implementation is not really meant for this use case, as it defines all the function in testlist.c (#define STANDALONE) + adds a main() and requires winetest_testlist. Thanks for the review :-) -- Cheers, Arek
On 5/3/21 6:37 PM, Arkadiusz Hiler wrote:
On Mon, May 03, 2021 at 06:06:35PM +0200, Piotr Caban wrote:
On 5/3/21 3:07 PM, Arkadiusz Hiler wrote:
+static unsigned internal_thread_proc(void *param) +{ + struct threaddll_args *args = param; + SetEvent(args->confirm_running); + WaitForSingleObject(args->past_free, 100); It doesn't matter that much in this case but please test WaitForSingleObject return value. It can still give some hints if tests start to fail.
Any standard way to do that from DLLs? The current winetest_*() implementation is not really meant for this use case, as it defines all the function in testlist.c (#define STANDALONE) + adds a main() and requires winetest_testlist. I don't know, probably it's not worth to invent something here. How about changing the timeout to INFINITE then?
participants (3)
-
Arkadiusz Hiler -
Dmitry Timoshkov -
Piotr Caban