[PATCH v6 0/7] MR9305: Reject pseudo-handles in NtWaitForMultipleObjects().
Some applications rely on this behavior. -- v6: kernel32/tests: Add more tests for waits on pseudo-handles. https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/kernelbase/sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/kernelbase/sync.c b/dlls/kernelbase/sync.c index 40aec41e5c8..31620a1aad2 100644 --- a/dlls/kernelbase/sync.c +++ b/dlls/kernelbase/sync.c @@ -438,7 +438,7 @@ DWORD WINAPI DECLSPEC_HOTPATCH WaitForMultipleObjectsEx( DWORD count, const HAND status = NtWaitForMultipleObjects( count, hloc, wait_all ? WaitAll : WaitAny, alertable, get_nt_timeout( &time, timeout ) ); - if (HIWORD(status)) /* is it an error code? */ + if (NT_ERROR(status)) { SetLastError( RtlNtStatusToDosError(status) ); status = WAIT_FAILED; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/kernelbase/sync.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/dlls/kernelbase/sync.c b/dlls/kernelbase/sync.c index 31620a1aad2..3948f7ea4bd 100644 --- a/dlls/kernelbase/sync.c +++ b/dlls/kernelbase/sync.c @@ -395,7 +395,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH UnregisterWaitEx( HANDLE handle, HANDLE event ) */ DWORD WINAPI DECLSPEC_HOTPATCH WaitForSingleObject( HANDLE handle, DWORD timeout ) { - return WaitForMultipleObjectsEx( 1, &handle, FALSE, timeout, FALSE ); + return WaitForSingleObjectEx( handle, timeout, FALSE ); } @@ -404,7 +404,18 @@ DWORD WINAPI DECLSPEC_HOTPATCH WaitForSingleObject( HANDLE handle, DWORD timeout */ DWORD WINAPI DECLSPEC_HOTPATCH WaitForSingleObjectEx( HANDLE handle, DWORD timeout, BOOL alertable ) { - return WaitForMultipleObjectsEx( 1, &handle, FALSE, timeout, alertable ); + NTSTATUS status; + LARGE_INTEGER time; + + status = NtWaitForSingleObject( normalize_std_handle( handle ), alertable, + get_nt_timeout( &time, timeout ) ); + + if (NT_ERROR(status)) + { + SetLastError( RtlNtStatusToDosError(status) ); + status = WAIT_FAILED; + } + return status; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
From: Marc-Aurel Zent <mzent(a)codeweavers.com> Also fixes an off-by-one error in the pseudo-handle check. --- dlls/ntdll/unix/sync.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 07ffe854104..ef6dfe1e2c1 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -556,6 +556,11 @@ static inline unsigned int inproc_sync_handle_to_index( HANDLE handle, unsigned return idx % INPROC_SYNC_CACHE_BLOCK_SIZE; } +static inline BOOL is_pseudo_handle( HANDLE handle ) +{ + return ((ULONG)(ULONG_PTR)handle >= 0xfffffffa); +} + static struct inproc_sync *cache_inproc_sync( HANDLE handle, struct inproc_sync *sync ) { unsigned int entry, idx = inproc_sync_handle_to_index( handle, &entry ); @@ -563,7 +568,7 @@ static struct inproc_sync *cache_inproc_sync( HANDLE handle, struct inproc_sync int refcount; /* don't cache pseudo-handles; waiting on them is pointless anyway */ - if ((ULONG)(ULONG_PTR)handle > 0xfffffffa) return sync; + if (is_pseudo_handle( handle )) return sync; if (entry >= INPROC_SYNC_CACHE_ENTRIES) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/ntdll/unix/sync.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index ef6dfe1e2c1..c07d318fd1e 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2338,7 +2338,24 @@ NTSTATUS WINAPI NtWaitForMultipleObjects( DWORD count, const HANDLE *handles, WA */ NTSTATUS WINAPI NtWaitForSingleObject( HANDLE handle, BOOLEAN alertable, const LARGE_INTEGER *timeout ) { - return NtWaitForMultipleObjects( 1, &handle, FALSE, alertable, timeout ); + union select_op select_op; + UINT flags = SELECT_INTERRUPTIBLE; + unsigned int ret; + + TRACE( "handle %p, alertable %u, timeout %s\n", handle, alertable, debugstr_timeout(timeout) ); + + if ((ret = inproc_wait( 1, &handle, WaitAny, alertable, timeout )) != STATUS_NOT_IMPLEMENTED) + { + TRACE( "-> %#x\n", ret ); + return ret; + } + + if (alertable) flags |= SELECT_ALERTABLE; + select_op.wait.op = SELECT_WAIT; + select_op.wait.handles[0] = wine_server_obj_handle( handle ); + ret = server_wait( &select_op, offsetof( union select_op, wait.handles[1] ), flags, timeout ); + TRACE( "-> %#x\n", ret ); + return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/kernel32/tests/sync.c | 16 ++++++++-------- dlls/ntdll/unix/sync.c | 5 +++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c index 466bbc8908b..30ea72907ef 100644 --- a/dlls/kernel32/tests/sync.c +++ b/dlls/kernel32/tests/sync.c @@ -1432,26 +1432,26 @@ static void test_WaitForMultipleObjects(void) maxevents[0] = GetCurrentProcess(); SetLastError(0xdeadbeef); r = WaitForMultipleObjects(1, maxevents, FALSE, 100); - todo_wine ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); - todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, - "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); + ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); + ok(GetLastError() == ERROR_INVALID_HANDLE, + "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); maxevents[0] = GetCurrentThread(); SetLastError(0xdeadbeef); r = WaitForMultipleObjects(1, maxevents, FALSE, 100); - todo_wine ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); - todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, - "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); + ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); + ok(GetLastError() == ERROR_INVALID_HANDLE, + "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); timeout.QuadPart = -1000000; maxevents[0] = GetCurrentProcess(); status = pNtWaitForMultipleObjects(1, maxevents, WaitAny, FALSE, &timeout); - todo_wine ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); timeout.QuadPart = -1000000; maxevents[0] = GetCurrentThread(); status = pNtWaitForMultipleObjects(1, maxevents, WaitAny, FALSE, &timeout); - todo_wine ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); } static BOOL g_initcallback_ret, g_initcallback_called; diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index c07d318fd1e..b080d61fcf5 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2318,6 +2318,11 @@ NTSTATUS WINAPI NtWaitForMultipleObjects( DWORD count, const HANDLE *handles, WA TRACE( "}, timeout %s\n", debugstr_timeout(timeout) ); } + /* Reject pseudo-handles up front. These are not valid for multi-object waits. */ + for (i = 0; i < count; i++) + if (is_pseudo_handle( handles[i] )) + return STATUS_INVALID_HANDLE; + if ((ret = inproc_wait( count, handles, type, alertable, timeout )) != STATUS_NOT_IMPLEMENTED) { TRACE( "-> %#x\n", ret ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- include/winbase.h | 36 +++++++++++++++++++++--------------- include/winternl.h | 8 ++++++-- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/include/winbase.h b/include/winbase.h index c96f58a61b3..4ae800bfc46 100644 --- a/include/winbase.h +++ b/include/winbase.h @@ -2657,6 +2657,9 @@ extern WCHAR * CDECL wine_get_dos_file_name( const char * ) __WINE_DEALLOC(HeapF #define GetCurrentProcess() NtCurrentProcess() #define GetCurrentThread() NtCurrentThread() +#define GetCurrentProcessToken() NtCurrentProcessToken() +#define GetCurrentThreadToken() NtCurrentThreadToken() +#define GetCurrentThreadEffectiveToken() NtCurrentEffectiveToken() #define GetCurrentProcessId() HandleToULong(NtCurrentTeb()->ClientId.UniqueProcess) #define GetCurrentThreadId() HandleToULong(NtCurrentTeb()->ClientId.UniqueThread) @@ -2682,6 +2685,21 @@ static FORCEINLINE DWORD WINAPI GetCurrentThreadId(void) return HandleToULong( ((HANDLE *)NtCurrentTeb())[9] ); } +static FORCEINLINE HANDLE WINAPI GetCurrentProcessToken(void) +{ + return (HANDLE)~(ULONG_PTR)3; +} + +static FORCEINLINE HANDLE WINAPI GetCurrentThreadToken(void) +{ + return (HANDLE)~(ULONG_PTR)4; +} + +static FORCEINLINE HANDLE WINAPI GetCurrentThreadEffectiveToken(void) +{ + return (HANDLE)~(ULONG_PTR)5; +} + static FORCEINLINE DWORD WINAPI GetLastError(void) { return *(DWORD *)((void **)NtCurrentTeb() + 13); @@ -2703,27 +2721,15 @@ WINBASEAPI HANDLE WINAPI GetCurrentProcess(void); WINBASEAPI DWORD WINAPI GetCurrentProcessId(void); WINBASEAPI HANDLE WINAPI GetCurrentThread(void); WINBASEAPI DWORD WINAPI GetCurrentThreadId(void); +WINBASEAPI HANDLE WINAPI GetCurrentProcessToken(void); +WINBASEAPI HANDLE WINAPI GetCurrentThreadToken(void); +WINBASEAPI HANDLE WINAPI GetCurrentThreadEffectiveToken(void); WINBASEAPI DWORD WINAPI GetLastError(void); WINBASEAPI HANDLE WINAPI GetProcessHeap(void); WINBASEAPI VOID WINAPI SetLastError(DWORD); #endif /* __WINESRC__ */ -static FORCEINLINE HANDLE WINAPI GetCurrentProcessToken(void) -{ - return (HANDLE)~(ULONG_PTR)3; -} - -static FORCEINLINE HANDLE WINAPI GetCurrentThreadToken(void) -{ - return (HANDLE)~(ULONG_PTR)4; -} - -static FORCEINLINE HANDLE WINAPI GetCurrentThreadEffectiveToken(void) -{ - return (HANDLE)~(ULONG_PTR)5; -} - /* WinMain(entry point) must be declared in winbase.h. */ /* If this is not declared, we cannot compile many sources written with C++. */ int WINAPI WinMain(HINSTANCE,HINSTANCE,LPSTR,int); diff --git a/include/winternl.h b/include/winternl.h index d94b9911c74..7dea51d2f93 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -5284,8 +5284,12 @@ NTSYSAPI NTSTATUS WINAPI RtlLargeIntegerToChar(const ULONGLONG *,ULONG,ULONG,PC * Inline functions */ -#define NtCurrentProcess() ((HANDLE)~(ULONG_PTR)0) -#define NtCurrentThread() ((HANDLE)~(ULONG_PTR)1) +#define NtCurrentProcess() ((HANDLE)~(ULONG_PTR)0) +#define NtCurrentThread() ((HANDLE)~(ULONG_PTR)1) +#define NtCurrentSession() ((HANDLE)~(ULONG_PTR)2) +#define NtCurrentProcessToken() ((HANDLE)~(ULONG_PTR)3) +#define NtCurrentThreadToken() ((HANDLE)~(ULONG_PTR)4) +#define NtCurrentEffectiveToken() ((HANDLE)~(ULONG_PTR)5) #define RtlFillMemory(Destination,Length,Fill) memset((Destination),(Fill),(Length)) #define RtlMoveMemory(Destination,Source,Length) memmove((Destination),(Source),(Length)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
From: Marc-Aurel Zent <mzent(a)codeweavers.com> Also improve some formatting in surrounding for-loops. --- dlls/kernel32/tests/sync.c | 181 +++++++++++++++++++++++++++++-------- 1 file changed, 142 insertions(+), 39 deletions(-) diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c index 30ea72907ef..74c5d1f08c2 100644 --- a/dlls/kernel32/tests/sync.c +++ b/dlls/kernel32/tests/sync.c @@ -1291,6 +1291,25 @@ static void test_WaitForSingleObject(void) LARGE_INTEGER timeout; NTSTATUS status; DWORD ret; + int i; + HANDLE waitable_pseudohandles[] = + { + NtCurrentProcess(), + NtCurrentThread(), + }; + HANDLE non_waitable_pseudohandles[] = + { + NtCurrentSession(), + NtCurrentProcessToken(), + NtCurrentThreadToken(), + NtCurrentEffectiveToken(), + }; + HANDLE std_handles[] = + { + (HANDLE)STD_INPUT_HANDLE, + (HANDLE)STD_OUTPUT_HANDLE, + (HANDLE)STD_ERROR_HANDLE, + }; signaled = CreateEventW(NULL, TRUE, TRUE, NULL); nonsignaled = CreateEventW(NULL, TRUE, FALSE, NULL); @@ -1359,20 +1378,44 @@ static void test_WaitForSingleObject(void) ok(ret == WAIT_OBJECT_0, "expected WAIT_OBJECT_0, got %ld\n", ret); ok(GetLastError() == 0xdeadbeef, "expected 0xdeadbeef, got %ld\n", GetLastError()); - /* pseudo handles are allowed in WaitForSingleObject and NtWaitForSingleObject */ - ret = WaitForSingleObject(GetCurrentProcess(), 100); - ok(ret == WAIT_TIMEOUT, "expected WAIT_TIMEOUT, got %lu\n", ret); + /* waitable pseudo-handles are allowed in WaitForSingleObject and NtWaitForSingleObject, + * but not in NtWaitForMultipleObjects, see test_WaitForMultipleObjects */ + for (i = 0; i < ARRAY_SIZE(waitable_pseudohandles); i++) + { + ret = WaitForSingleObject(waitable_pseudohandles[i], 0); + ok(ret == WAIT_TIMEOUT, "expected WAIT_TIMEOUT, got %lu\n", ret); - ret = WaitForSingleObject(GetCurrentThread(), 100); - ok(ret == WAIT_TIMEOUT, "expected WAIT_TIMEOUT, got %lu\n", ret); + timeout.QuadPart = 0; + status = pNtWaitForSingleObject(waitable_pseudohandles[i], FALSE, &timeout); + ok(status == STATUS_TIMEOUT, "expected STATUS_TIMEOUT, got %08lx\n", status); + } - timeout.QuadPart = -1000000; - status = pNtWaitForSingleObject(GetCurrentProcess(), FALSE, &timeout); - ok(status == STATUS_TIMEOUT, "expected STATUS_TIMEOUT, got %08lx\n", status); + /* non-waitable pseudo-handles return STATUS_INVALID_HANDLE */ + for (i = 0; i < ARRAY_SIZE(non_waitable_pseudohandles); i++) + { + SetLastError(0xdeadbeef); + ret = WaitForSingleObject(non_waitable_pseudohandles[i], 0); + ok(ret == WAIT_FAILED, "expected WAIT_FAILED, got %ld\n", ret); + ok(GetLastError() == ERROR_INVALID_HANDLE, "expected ERROR_INVALID_HANDLE, got %ld\n", GetLastError()); + + status = pNtWaitForSingleObject(non_waitable_pseudohandles[i], FALSE, NULL); + todo_wine_if((non_waitable_pseudohandles[i] == NtCurrentProcessToken() || + non_waitable_pseudohandles[i] == NtCurrentEffectiveToken()) && + status == STATUS_OBJECT_TYPE_MISMATCH) + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + } - timeout.QuadPart = -1000000; - status = pNtWaitForSingleObject(GetCurrentThread(), FALSE, &timeout); - ok(status == STATUS_TIMEOUT, "expected STATUS_TIMEOUT, got %08lx\n", status); + /* std handles are only allowed in WaitForSingleObject and not NtWaitForSingleObject */ + for (i = 0; i < ARRAY_SIZE(std_handles); i++) + { + ret = WaitForSingleObject(std_handles[i], 100); + ok(ret == WAIT_OBJECT_0 || ret == WAIT_TIMEOUT, + "expected WAIT_OBJECT_0 or WAIT_TIMEOUT), got %lu, GetLastError()=%lu\n", + ret, GetLastError()); + + status = pNtWaitForSingleObject(std_handles[i], FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + } CloseHandle(signaled); CloseHandle(nonsignaled); @@ -1380,15 +1423,29 @@ static void test_WaitForSingleObject(void) static void test_WaitForMultipleObjects(void) { - LARGE_INTEGER timeout; NTSTATUS status; DWORD r; int i; HANDLE maxevents[MAXIMUM_WAIT_OBJECTS]; + HANDLE pseudohandles[] = + { + NtCurrentProcess(), + NtCurrentThread(), + NtCurrentSession(), + NtCurrentProcessToken(), + NtCurrentThreadToken(), + NtCurrentEffectiveToken(), + }; + HANDLE std_handles[] = + { + (HANDLE)STD_INPUT_HANDLE, + (HANDLE)STD_OUTPUT_HANDLE, + (HANDLE)STD_ERROR_HANDLE, + }; /* create the maximum number of events and make sure * we can wait on that many */ - for (i=0; i<MAXIMUM_WAIT_OBJECTS; i++) + for (i = 0; i < MAXIMUM_WAIT_OBJECTS; i++) { maxevents[i] = CreateEventW(NULL, i==0, TRUE, NULL); ok( maxevents[i] != 0, "should create enough events\n"); @@ -1400,7 +1457,7 @@ static void test_WaitForMultipleObjects(void) r = WaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, FALSE, 0); ok( r == WAIT_OBJECT_0, "should signal handle #0 first, got %ld\n", r); ok(ResetEvent(maxevents[0]), "ResetEvent\n"); - for (i=1; i<MAXIMUM_WAIT_OBJECTS; i++) + for (i = 1; i < MAXIMUM_WAIT_OBJECTS; i++) { /* the lowest index is checked first and remaining events are untouched */ r = WaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, FALSE, 0); @@ -1408,7 +1465,7 @@ static void test_WaitForMultipleObjects(void) } /* run same test with Nt* call */ - for (i=0; i<MAXIMUM_WAIT_OBJECTS; i++) + for (i = 0; i < MAXIMUM_WAIT_OBJECTS; i++) SetEvent(maxevents[i]); /* a manual-reset event remains signaled, an auto-reset event is cleared */ @@ -1417,41 +1474,87 @@ static void test_WaitForMultipleObjects(void) status = pNtWaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, WaitAny, FALSE, NULL); ok(status == STATUS_WAIT_0, "should signal handle #0 first, got %08lx\n", status); ok(ResetEvent(maxevents[0]), "ResetEvent\n"); - for (i=1; i<MAXIMUM_WAIT_OBJECTS; i++) + for (i = 1; i < MAXIMUM_WAIT_OBJECTS; i++) { /* the lowest index is checked first and remaining events are untouched */ status = pNtWaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, WaitAny, FALSE, NULL); ok(status == STATUS_WAIT_0 + i, "should signal handle #%d first, got %08lx\n", i, status); } - for (i=0; i<MAXIMUM_WAIT_OBJECTS; i++) - if (maxevents[i]) CloseHandle(maxevents[i]); + CloseHandle(maxevents[0]); - /* in contrast to WaitForSingleObject, pseudo handles are not allowed in + /* in contrast to WaitForSingleObject, all pseudo-handles are not allowed in * WaitForMultipleObjects and NtWaitForMultipleObjects */ - maxevents[0] = GetCurrentProcess(); - SetLastError(0xdeadbeef); - r = WaitForMultipleObjects(1, maxevents, FALSE, 100); - ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); - ok(GetLastError() == ERROR_INVALID_HANDLE, - "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); + for (i = 0; i < ARRAY_SIZE(pseudohandles); i++) + { + maxevents[0] = pseudohandles[i]; + SetLastError(0xdeadbeef); + r = WaitForMultipleObjects(1, maxevents, FALSE, 0); + ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); + ok(GetLastError() == ERROR_INVALID_HANDLE, + "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); - maxevents[0] = GetCurrentThread(); - SetLastError(0xdeadbeef); - r = WaitForMultipleObjects(1, maxevents, FALSE, 100); - ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); - ok(GetLastError() == ERROR_INVALID_HANDLE, - "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); + status = pNtWaitForMultipleObjects(1, maxevents, WaitAny, FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + + /* also not allowed with a wait count greater than 1 with both WaitAny and WaitAll */ + SetLastError(0xdeadbeef); + r = WaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, FALSE, 0); + ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); + ok(GetLastError() == ERROR_INVALID_HANDLE, + "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); + SetLastError(0xdeadbeef); + r = WaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, TRUE, 0); + ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); + ok(GetLastError() == ERROR_INVALID_HANDLE, + "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); - timeout.QuadPart = -1000000; - maxevents[0] = GetCurrentProcess(); - status = pNtWaitForMultipleObjects(1, maxevents, WaitAny, FALSE, &timeout); - ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + status = pNtWaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, WaitAny, FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + status = pNtWaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, WaitAll, FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); - timeout.QuadPart = -1000000; - maxevents[0] = GetCurrentThread(); - status = pNtWaitForMultipleObjects(1, maxevents, WaitAny, FALSE, &timeout); - ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + /* irrespective of the index that contains the pseudo handle for both wait types */ + maxevents[0] = maxevents[MAXIMUM_WAIT_OBJECTS - 1]; + maxevents[MAXIMUM_WAIT_OBJECTS - 1] = pseudohandles[i]; + SetLastError(0xdeadbeef); + r = WaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, FALSE, 0); + ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); + ok(GetLastError() == ERROR_INVALID_HANDLE, + "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); + SetLastError(0xdeadbeef); + r = WaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, TRUE, 0); + ok(r == WAIT_FAILED, "expected WAIT_FAILED, got %lu\n", r); + ok(GetLastError() == ERROR_INVALID_HANDLE, + "expected ERROR_INVALID_HANDLE, got %lu\n", GetLastError()); + + status = pNtWaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, WaitAny, FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + status = pNtWaitForMultipleObjects(MAXIMUM_WAIT_OBJECTS, maxevents, WaitAll, FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + maxevents[MAXIMUM_WAIT_OBJECTS - 1] = maxevents[0]; + } + + /* Similar to WaitForSingleObject std handles are only allowed in WaitForMultipleObjects and + * not NtWaitForMultipleObjects (for brevity we only test single count waits here) */ + for (i = 0; i < ARRAY_SIZE(std_handles); i++) + { + maxevents[0] = std_handles[i]; + SetLastError(0xdeadbeef); + r = WaitForMultipleObjects(1, maxevents, FALSE, 0); + ok(r == WAIT_OBJECT_0 || r == WAIT_TIMEOUT, + "expected WAIT_OBJECT_0 or WAIT_TIMEOUT), got %lu, GetLastError()=%lu\n", + r, GetLastError()); + + status = pNtWaitForMultipleObjects(1, maxevents, WaitAny, FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + + status = pNtWaitForMultipleObjects(1, maxevents, WaitAll, FALSE, NULL); + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + } + + for (i = 1; i < MAXIMUM_WAIT_OBJECTS; i++) + if (maxevents[i]) CloseHandle(maxevents[i]); } static BOOL g_initcallback_ret, g_initcallback_called; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9305
On Thu Oct 30 23:53:37 2025 +0000, Marc-Aurel Zent wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/9305/diffs?diff_id=219982&start_sha=9cc80ae4ceb93db13f519bacba319f5c3f72daeb#bde7f180050ff4134043396571b3f8d20aba512e_1295_1295) Ah I overlooked this comment unfortunately...
Fixed in v6. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120224
On Thu Oct 30 23:55:12 2025 +0000, Paul Gofman wrote:
Indentation (also in other places), I think it should either align to GetLastError() start or have two tabs from 'ok' start. Fixed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120225
On Thu Oct 30 23:19:01 2025 +0000, Marc-Aurel Zent wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/9305/diffs?diff_id=219969&start_sha=58d355666a5f27d456e5a50685ed490226463603#6a31f4cf2c4d24038e67fe3417ad92c8f1f6cfdc_2319_2323) I added a small helper.
Also, probably '\> 0xfffffffa' as in cache_inproc_sync()?
That was actually a mistake there that would not catch `NtCurrentEffectiveToken()`, although pretty harmless ultimately in `cache_inproc_sync()`, where it just causes a `FIXME()`. For us here it would cause a bunch of test failures though... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120226
On Fri Oct 31 00:41:32 2025 +0000, Marc-Aurel Zent wrote:
We could use `GetCurrentProcessToken()`, but `NtCurrentProcessToken()` is a fixed compile time constant instead of a function and is a bit more pleasant to use (especially to initialize arrays). I also think it would be nice to have both the NT versions and Win32 ones for all of the pseudo-handles for consistencies sake. I am so far failing to see practical difference, that is inline function defined in header now:
static FORCEINLINE HANDLE WINAPI GetCurrentProcessToken(void)
{
return (HANDLE)~(ULONG_PTR)3;
}
In practice for any *relatively* new compiler (as in less than a couple of decades old at least probably), there is no slightest performance difference between this and '#define constant', the constant will be inlined either way. The only difference in having '#define' could be for initializing static variables but I doubt this is an actual usecase. I think the other handles exist as Nt...() because they are frequently used in Wine Unix libs (and you don't add such usage in the patches for tokens). Even there, in ntdll Unix part, there are a lot of usages of GetCurrentProcess() besides NtCurrentProcess(), I don't think that matters much really. And as soon as it doesn't matter and new functions are not used IMO it makes sense not to introduce unneeded changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120229
Paul Gofman (@gofman) commented about dlls/ntdll/unix/sync.c:
return idx % INPROC_SYNC_CACHE_BLOCK_SIZE; }
+static inline BOOL is_pseudo_handle( HANDLE handle )
Very minor and may be used elsewhere, but 'inline' qualifier is useless these days. same-file functions will be inlined either way if compiler decides so (surely will in this case). And in some case (not this one probably) when it would decide not to might be questionable if forcing inlining is useful (in some other specific case). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120230
Paul Gofman (@gofman) commented about include/winbase.h:
WINBASEAPI DWORD WINAPI GetCurrentProcessId(void); WINBASEAPI HANDLE WINAPI GetCurrentThread(void); WINBASEAPI DWORD WINAPI GetCurrentThreadId(void); +WINBASEAPI HANDLE WINAPI GetCurrentProcessToken(void); I still think these changes are not needed, at least until token functions are extensively used on the Wine Unix side.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120231
Paul Gofman (@gofman) commented about dlls/kernel32/tests/sync.c:
+ ok(GetLastError() == ERROR_INVALID_HANDLE, "expected ERROR_INVALID_HANDLE, got %ld\n", GetLastError()); + + status = pNtWaitForSingleObject(non_waitable_pseudohandles[i], FALSE, NULL); + todo_wine_if((non_waitable_pseudohandles[i] == NtCurrentProcessToken() || + non_waitable_pseudohandles[i] == NtCurrentEffectiveToken()) && + status == STATUS_OBJECT_TYPE_MISMATCH) + ok(status == STATUS_INVALID_HANDLE, "expected STATUS_INVALID_HANDLE, got %08lx\n", status); + }
- timeout.QuadPart = -1000000; - status = pNtWaitForSingleObject(GetCurrentThread(), FALSE, &timeout); - ok(status == STATUS_TIMEOUT, "expected STATUS_TIMEOUT, got %08lx\n", status); + /* std handles are only allowed in WaitForSingleObject and not NtWaitForSingleObject */ + for (i = 0; i < ARRAY_SIZE(std_handles); i++) + { + ret = WaitForSingleObject(std_handles[i], 100); We don't need wait 100ms here if it is going to timeout, let it be 0 and timeout or succeed at once not to waste test time.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120232
Paul Gofman (@gofman) commented about dlls/ntdll/unix/sync.c:
TRACE( "}, timeout %s\n", debugstr_timeout(timeout) ); }
+ /* Reject pseudo-handles up front. These are not valid for multi-object waits. */ + for (i = 0; i < count; i++) + if (is_pseudo_handle( handles[i] )) + return STATUS_INVALID_HANDLE;
we use to always put loop body in '{' '}' (each on separate line). while 'return STATUS_INVALID_HANDLE;' can go into the same 'if' line (well, at least in ntdll / kernelbase :). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9305#note_120233
participants (3)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠mzent)
-
Paul Gofman (＠gofman)