[PATCH 1/2] kernel32/tests: Trace thread IDs in hexadecimal.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/kernel32/tests/loader.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 1f6f317..6d532eb 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -1523,7 +1523,7 @@ static DWORD WINAPI mutex_thread_proc(void *param) wait_list[2] = peb_lock_event; wait_list[3] = heap_lock_event; - trace("%04u: mutex_thread_proc: starting\n", GetCurrentThreadId()); + trace("%04x: mutex_thread_proc: starting\n", GetCurrentThreadId()); while (1) { ret = WaitForMultipleObjects(sizeof(wait_list)/sizeof(wait_list[0]), wait_list, FALSE, 50); @@ -1531,7 +1531,7 @@ static DWORD WINAPI mutex_thread_proc(void *param) else if (ret == WAIT_OBJECT_0 + 1) { ULONG_PTR loader_lock_magic; - trace("%04u: mutex_thread_proc: Entering loader lock\n", GetCurrentThreadId()); + trace("%04x: mutex_thread_proc: Entering loader lock\n", GetCurrentThreadId()); ret = pLdrLockLoaderLock(0, NULL, &loader_lock_magic); ok(!ret, "LdrLockLoaderLock error %#x\n", ret); inside_loader_lock++; @@ -1539,21 +1539,21 @@ static DWORD WINAPI mutex_thread_proc(void *param) } else if (ret == WAIT_OBJECT_0 + 2) { - trace("%04u: mutex_thread_proc: Entering PEB lock\n", GetCurrentThreadId()); + trace("%04x: mutex_thread_proc: Entering PEB lock\n", GetCurrentThreadId()); pRtlAcquirePebLock(); inside_peb_lock++; SetEvent(ack_event); } else if (ret == WAIT_OBJECT_0 + 3) { - trace("%04u: mutex_thread_proc: Entering heap lock\n", GetCurrentThreadId()); + trace("%04x: mutex_thread_proc: Entering heap lock\n", GetCurrentThreadId()); HeapLock(GetProcessHeap()); inside_heap_lock++; SetEvent(ack_event); } } - trace("%04u: mutex_thread_proc: exiting\n", GetCurrentThreadId()); + trace("%04x: mutex_thread_proc: exiting\n", GetCurrentThreadId()); return 196; } @@ -1569,11 +1569,11 @@ static DWORD WINAPI semaphore_thread_proc(void *param) while (1) { if (winetest_debug > 1) - trace("%04u: semaphore_thread_proc: still alive\n", GetCurrentThreadId()); + trace("%04x: semaphore_thread_proc: still alive\n", GetCurrentThreadId()); if (WaitForSingleObject(stop_event, 50) != WAIT_TIMEOUT) break; } - trace("%04u: semaphore_thread_proc: exiting\n", GetCurrentThreadId()); + trace("%04x: semaphore_thread_proc: exiting\n", GetCurrentThreadId()); return 196; } @@ -1585,7 +1585,7 @@ static DWORD WINAPI noop_thread_proc(void *param) InterlockedIncrement(noop_thread_started); } - trace("%04u: noop_thread_proc: exiting\n", GetCurrentThreadId()); + trace("%04x: noop_thread_proc: exiting\n", GetCurrentThreadId()); return 195; } -- 2.7.4
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- Not only were we leaking handles, we previously closed the stop_event handle immediately after setting it, so mutex_thread_proc would sometimes loop forever trying to wait on an invalid handle, causing an intermittent failure both on Wine and on Windows. This patch fixes that failure. dlls/kernel32/tests/loader.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 6d532eb..da24c80 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -1752,6 +1752,10 @@ static BOOL WINAPI dll_entry_point(HINSTANCE hinst, DWORD reason, LPVOID param) ret = WaitForSingleObject(semaphore, 0); ok(ret == WAIT_TIMEOUT, "expected WAIT_TIMEOUT, got %#x\n", ret); + CloseHandle(event); + CloseHandle(mutex); + CloseHandle(semaphore); + if (expected_code == STILL_ACTIVE) { ret = WaitForSingleObject(attached_thread[0], 0); @@ -2149,7 +2153,6 @@ static void child_process(const char *dll_name, DWORD target_offset) case 3: trace("signalling thread exit\n"); SetEvent(stop_event); - CloseHandle(stop_event); break; case 4: @@ -2237,6 +2240,15 @@ static void child_process(const char *dll_name, DWORD target_offset) *child_failures = winetest_get_failures(); + CloseHandle(loader_lock_event); + CloseHandle(peb_lock_event); + CloseHandle(heap_lock_event); + CloseHandle(ack_event); + CloseHandle(stop_event); + CloseHandle(process); + /* Don't close event, mutex and semaphore here; we want to test if they are + * abandoned on process termination. */ + trace("call ExitProcess(195)\n"); ExitProcess(195); } -- 2.7.4
Zebediah Figura <z.figura12(a)gmail.com> wrote:
@@ -2149,7 +2153,6 @@ static void child_process(const char *dll_name, DWORD target_offset) case 3: trace("signalling thread exit\n"); SetEvent(stop_event); - CloseHandle(stop_event); break;
This one is a real bug, thanks for tracking it down.
+ CloseHandle(loader_lock_event); + CloseHandle(peb_lock_event); + CloseHandle(heap_lock_event); + CloseHandle(ack_event); + CloseHandle(stop_event); + CloseHandle(process); + /* Don't close event, mutex and semaphore here; we want to test if they are + * abandoned on process termination. */ + trace("call ExitProcess(195)\n"); ExitProcess(195); }
Please don't add these CloseHandle() calls, they clutter the code and don't do anything useful. -- Dmitry.
On 11/16/2017 09:56 PM, Dmitry Timoshkov wrote:
+ CloseHandle(loader_lock_event); + CloseHandle(peb_lock_event); + CloseHandle(heap_lock_event); + CloseHandle(ack_event); + CloseHandle(stop_event); + CloseHandle(process); + /* Don't close event, mutex and semaphore here; we want to test if they are + * abandoned on process termination. */ + trace("call ExitProcess(195)\n"); ExitProcess(195); }
Please don't add these CloseHandle() calls, they clutter the code and don't do anything useful.
Thanks. I've resent the patch just to remove the existing CloseHandle() call.
participants (2)
-
Dmitry Timoshkov -
Zebediah Figura