[PATCH 1/2] kernel32/tests: Trace thread IDs in hexadecimal.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 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;
 }

[PATCH 2/2] kernel32/tests: Close all handles on process exit.

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
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);
 }

[PATCH 2/2] kernel32/tests: Close all handles on process exit.

[Dmitry Timoshkov]

Zebediah Figura  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.

[PATCH 2/2] kernel32/tests: Close all handles on process exit.

[Zebediah Figura]

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.