[PATCH 0/3] MR8572: kernelbase: Always wait in GetOverlappedResultEx() if timeout is nonzero.
From: Paul Gofman pgofman@codeweavers.com
--- dlls/kernel32/tests/file.c | 189 +++++++++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 83766dfaaa2..e4ec612d487 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -38,6 +38,7 @@ #include "ddk/ntifs.h"
static HANDLE (WINAPI *pFindFirstFileExA)(LPCSTR,FINDEX_INFO_LEVELS,LPVOID,FINDEX_SEARCH_OPS,LPVOID,DWORD); +static BOOL (WINAPI *pGetOverlappedResultEx)(HANDLE, OVERLAPPED *, DWORD *, DWORD, BOOL); static BOOL (WINAPI *pReplaceFileW)(LPCWSTR, LPCWSTR, LPCWSTR, DWORD, LPVOID, LPVOID); static UINT (WINAPI *pGetSystemWindowsDirectoryA)(LPSTR, UINT); static BOOL (WINAPI *pGetVolumeNameForVolumeMountPointA)(LPCSTR, LPSTR, DWORD); @@ -93,6 +94,7 @@ static void InitFunctionPointers(void) pRtlFreeUnicodeString = (void *)GetProcAddress(hntdll, "RtlFreeUnicodeString");
pFindFirstFileExA=(void*)GetProcAddress(hkernel32, "FindFirstFileExA"); + pGetOverlappedResultEx =(void*)GetProcAddress(hkernel32, "GetOverlappedResultEx"); pReplaceFileW=(void*)GetProcAddress(hkernel32, "ReplaceFileW"); pGetSystemWindowsDirectoryA=(void*)GetProcAddress(hkernel32, "GetSystemWindowsDirectoryA"); pGetVolumeNameForVolumeMountPointA = (void *) GetProcAddress(hkernel32, "GetVolumeNameForVolumeMountPointA"); @@ -3739,6 +3741,7 @@ static void test_overlapped(void) { OVERLAPPED ov; DWORD r, result; + HANDLE event;
/* GetOverlappedResult crashes if the 2nd or 3rd param are NULL */ if (0) /* tested: WinXP */ @@ -3810,6 +3813,192 @@ static void test_overlapped(void)
r = CloseHandle( ov.hEvent ); ok( r == TRUE, "close handle failed\n"); + + if (!pGetOverlappedResultEx) + { + win_skip( "GetOverlappedResultEx is not available, skipping tests.\n" ); + return; + } + + event = CreateEventW(NULL, FALSE, FALSE, NULL); + + result = 0xdeadbeef; + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + r = pGetOverlappedResultEx(event, &ov, &result, 1, FALSE); + ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + r = GetOverlappedResult(event, &ov, &result, FALSE); + ok(!r && GetLastError() == ERROR_IO_INCOMPLETE, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + SetLastError( 0xdeadbeef ); + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + QueueUserAPC( user_apc, GetCurrentThread(), 0 ); + r = GetOverlappedResult(event, &ov, &result, FALSE); + ok(!r && GetLastError() == ERROR_IO_INCOMPLETE, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + SetEvent(event); + QueueUserAPC( user_apc, GetCurrentThread(), 0 ); + r = GetOverlappedResult(event, &ov, &result, TRUE); + ok(r, "got %lu, error %lu.\n", r, GetLastError()); + r = WaitForSingleObject(event, 0); + ok(r == WAIT_TIMEOUT, "got %#lx.\n", r); + ok( result == 0xabcd, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + SetLastError( 0xdeadbeef ); + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + r = pGetOverlappedResultEx(event, &ov, &result, 0, FALSE); + ok(!r && GetLastError() == ERROR_IO_INCOMPLETE, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + SetLastError( 0xdeadbeef ); + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + SetEvent(event); + r = pGetOverlappedResultEx(event, &ov, &result, 0, FALSE); + ok(!r && GetLastError() == ERROR_IO_INCOMPLETE, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + r = WaitForSingleObject(event, 0); + ok(r == WAIT_OBJECT_0, "got %#lx.\n", r); + + result = 0xdeadbeef; + SetLastError( 0xdeadbeef ); + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + r = pGetOverlappedResultEx(event, &ov, &result, 1, FALSE); + ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + SetLastError( 0xdeadbeef ); + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = (HANDLE)0xdeadbeef; + r = pGetOverlappedResultEx(event, &ov, &result, 1, FALSE); + ok(!r && GetLastError() == ERROR_INVALID_HANDLE, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + SetLastError( 0xdeadbeef ); + ov.Internal = STATUS_PENDING; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + SetEvent(event); + r = pGetOverlappedResultEx(event, &ov, &result, 1, FALSE); + ok(ov.Internal == STATUS_PENDING, "got %#Ix.\n", ov.Internal); + ok(r, "got %lu, error %lu.\n", r, GetLastError()); + todo_wine ok(GetLastError() == ERROR_IO_PENDING, "got %lu.\n", GetLastError()); + ok( result == 0xabcd, "wrong result %lu\n", result ); + r = WaitForSingleObject(event, 0); + ok(r == WAIT_TIMEOUT, "got %#lx.\n", r); + + /* Test event wait if status is not STATUS_PENDING. */ + + /* With GetOverlapped result wait is skipped. */ + result = 0xdeadbeef; + ov.Internal = STATUS_SUCCESS; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + r = GetOverlappedResult(event, &ov, &result, TRUE); + ok(r, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xabcd, "wrong result %lu\n", result ); + + /* Unlike GetOverlappedResult, GetOverlappedResultEx always waits for event unless timeout is 0 even if + * ov.Internal is not STATUS_PENDING. */ + result = 0xdeadbeef; + ov.Internal = STATUS_SUCCESS; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + QueueUserAPC( user_apc, GetCurrentThread(), 0 ); + r = pGetOverlappedResultEx(event, &ov, &result, INFINITE, TRUE); + todo_wine ok(!r && GetLastError() == WAIT_IO_COMPLETION, "got %lu, error %lu.\n", r, GetLastError()); + + SetLastError(0xdeadbeef); + result = 0xdeadbeef; + ov.Internal = STATUS_SUCCESS; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + r = pGetOverlappedResultEx(event, &ov, &result, 0, 0); + todo_wine ok(GetLastError() == ERROR_SUCCESS, "got %lu.\n", GetLastError()); + ok(r, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xabcd, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + ov.Internal = STATUS_SUCCESS; + ov.InternalHigh = 0xabcd; + ov.hEvent = event; + r = pGetOverlappedResultEx(event, &ov, &result, 1, 0); + todo_wine ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + ov.Internal = STATUS_SUCCESS; + ov.InternalHigh = 0xabcd; + ov.hEvent = NULL; + QueueUserAPC( user_apc, GetCurrentThread(), 0 ); + r = pGetOverlappedResultEx(event, &ov, &result, INFINITE, TRUE); + todo_wine ok(!r && GetLastError() == WAIT_IO_COMPLETION, "got %lu, error %lu.\n", r, GetLastError()); + todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + SetLastError(0xdeadbeef); + result = 0xdeadbeef; + ov.Internal = STATUS_SUCCESS; + ov.InternalHigh = 0xabcd; + ov.hEvent = NULL; + r = pGetOverlappedResultEx(event, &ov, &result, 0, 0); + todo_wine ok(GetLastError() == ERROR_SUCCESS, "got %lu.\n", GetLastError()); + ok(r, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xabcd, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + ov.Internal = STATUS_SUCCESS; + ov.InternalHigh = 0xabcd; + ov.hEvent = NULL; + r = pGetOverlappedResultEx(event, &ov, &result, 1, 0); + todo_wine ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + ov.Internal = STATUS_UNEXPECTED_IO_ERROR; + ov.InternalHigh = 0xabcd; + ov.hEvent = NULL; + r = pGetOverlappedResultEx(event, &ov, &result, 1, 0); + todo_wine ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + + result = 0xdeadbeef; + ov.Internal = STATUS_UNEXPECTED_IO_ERROR; + ov.InternalHigh = 0xabcd; + ov.hEvent = NULL; + SetEvent(event); + r = pGetOverlappedResultEx(event, &ov, &result, 1, 0); + ok(!r && GetLastError() == ERROR_IO_DEVICE, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xabcd, "wrong result %lu\n", result ); + r = WaitForSingleObject(event, 0); + todo_wine ok(r == WAIT_TIMEOUT, "got %#lx.\n", r); + + CloseHandle(event); }
static void test_RemoveDirectory(void)
From: Paul Gofman pgofman@codeweavers.com
--- dlls/kernelbase/file.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c index 213cb926d7c..2435c174037 100644 --- a/dlls/kernelbase/file.c +++ b/dlls/kernelbase/file.c @@ -3212,7 +3212,38 @@ DWORD WINAPI DECLSPEC_HOTPATCH GetFileType( HANDLE file ) BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResult( HANDLE file, LPOVERLAPPED overlapped, LPDWORD result, BOOL wait ) { - return GetOverlappedResultEx( file, overlapped, result, wait ? INFINITE : 0, FALSE ); + NTSTATUS status; + DWORD ret; + + TRACE( "(%p %p %p %d)\n", file, overlapped, result, wait ); + + /* Paired with the write-release in set_async_iosb() in ntdll; see the + * latter for details. */ + status = ReadAcquire( (LONG *)&overlapped->Internal ); + if (status == STATUS_PENDING) + { + if (!wait) + { + SetLastError( ERROR_IO_INCOMPLETE ); + return FALSE; + } + ret = WaitForSingleObject( overlapped->hEvent ? overlapped->hEvent : file, INFINITE ); + if (ret == WAIT_FAILED) + return FALSE; + else if (ret) + { + SetLastError( ret ); + return FALSE; + } + + /* We don't need to give this load acquire semantics; the wait above + * already guarantees that the IOSB and output buffer are filled. */ + status = overlapped->Internal; + if (status == STATUS_PENDING) status = STATUS_SUCCESS; + } + + *result = overlapped->InternalHigh; + return set_ntstatus( status ); }
From: Paul Gofman pgofman@codeweavers.com
--- dlls/kernel32/tests/file.c | 26 +++++++++++++------------- dlls/kernelbase/file.c | 26 +++++++++++++------------- 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index e4ec612d487..040c5aa36bb 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -3908,7 +3908,7 @@ static void test_overlapped(void) r = pGetOverlappedResultEx(event, &ov, &result, 1, FALSE); ok(ov.Internal == STATUS_PENDING, "got %#Ix.\n", ov.Internal); ok(r, "got %lu, error %lu.\n", r, GetLastError()); - todo_wine ok(GetLastError() == ERROR_IO_PENDING, "got %lu.\n", GetLastError()); + ok(GetLastError() == ERROR_IO_PENDING, "got %lu.\n", GetLastError()); ok( result == 0xabcd, "wrong result %lu\n", result ); r = WaitForSingleObject(event, 0); ok(r == WAIT_TIMEOUT, "got %#lx.\n", r); @@ -3932,7 +3932,7 @@ static void test_overlapped(void) ov.hEvent = event; QueueUserAPC( user_apc, GetCurrentThread(), 0 ); r = pGetOverlappedResultEx(event, &ov, &result, INFINITE, TRUE); - todo_wine ok(!r && GetLastError() == WAIT_IO_COMPLETION, "got %lu, error %lu.\n", r, GetLastError()); + ok(!r && GetLastError() == WAIT_IO_COMPLETION, "got %lu, error %lu.\n", r, GetLastError());
SetLastError(0xdeadbeef); result = 0xdeadbeef; @@ -3940,7 +3940,7 @@ static void test_overlapped(void) ov.InternalHigh = 0xabcd; ov.hEvent = event; r = pGetOverlappedResultEx(event, &ov, &result, 0, 0); - todo_wine ok(GetLastError() == ERROR_SUCCESS, "got %lu.\n", GetLastError()); + ok(GetLastError() == ERROR_SUCCESS, "got %lu.\n", GetLastError()); ok(r, "got %lu, error %lu.\n", r, GetLastError()); ok( result == 0xabcd, "wrong result %lu\n", result );
@@ -3949,8 +3949,8 @@ static void test_overlapped(void) ov.InternalHigh = 0xabcd; ov.hEvent = event; r = pGetOverlappedResultEx(event, &ov, &result, 1, 0); - todo_wine ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); - todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result );
result = 0xdeadbeef; ov.Internal = STATUS_SUCCESS; @@ -3958,8 +3958,8 @@ static void test_overlapped(void) ov.hEvent = NULL; QueueUserAPC( user_apc, GetCurrentThread(), 0 ); r = pGetOverlappedResultEx(event, &ov, &result, INFINITE, TRUE); - todo_wine ok(!r && GetLastError() == WAIT_IO_COMPLETION, "got %lu, error %lu.\n", r, GetLastError()); - todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + ok(!r && GetLastError() == WAIT_IO_COMPLETION, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result );
SetLastError(0xdeadbeef); result = 0xdeadbeef; @@ -3967,7 +3967,7 @@ static void test_overlapped(void) ov.InternalHigh = 0xabcd; ov.hEvent = NULL; r = pGetOverlappedResultEx(event, &ov, &result, 0, 0); - todo_wine ok(GetLastError() == ERROR_SUCCESS, "got %lu.\n", GetLastError()); + ok(GetLastError() == ERROR_SUCCESS, "got %lu.\n", GetLastError()); ok(r, "got %lu, error %lu.\n", r, GetLastError()); ok( result == 0xabcd, "wrong result %lu\n", result );
@@ -3976,16 +3976,16 @@ static void test_overlapped(void) ov.InternalHigh = 0xabcd; ov.hEvent = NULL; r = pGetOverlappedResultEx(event, &ov, &result, 1, 0); - todo_wine ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); - todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result );
result = 0xdeadbeef; ov.Internal = STATUS_UNEXPECTED_IO_ERROR; ov.InternalHigh = 0xabcd; ov.hEvent = NULL; r = pGetOverlappedResultEx(event, &ov, &result, 1, 0); - todo_wine ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); - todo_wine ok( result == 0xdeadbeef, "wrong result %lu\n", result ); + ok(!r && GetLastError() == WAIT_TIMEOUT, "got %lu, error %lu.\n", r, GetLastError()); + ok( result == 0xdeadbeef, "wrong result %lu\n", result );
result = 0xdeadbeef; ov.Internal = STATUS_UNEXPECTED_IO_ERROR; @@ -3996,7 +3996,7 @@ static void test_overlapped(void) ok(!r && GetLastError() == ERROR_IO_DEVICE, "got %lu, error %lu.\n", r, GetLastError()); ok( result == 0xabcd, "wrong result %lu\n", result ); r = WaitForSingleObject(event, 0); - todo_wine ok(r == WAIT_TIMEOUT, "got %#lx.\n", r); + ok(r == WAIT_TIMEOUT, "got %#lx.\n", r);
CloseHandle(event); } diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c index 2435c174037..819a49063bf 100644 --- a/dlls/kernelbase/file.c +++ b/dlls/kernelbase/file.c @@ -3258,16 +3258,8 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file, OVERLAPPED *ov
TRACE( "(%p %p %p %lu %d)\n", file, overlapped, result, timeout, alertable );
- /* Paired with the write-release in set_async_iosb() in ntdll; see the - * latter for details. */ - status = ReadAcquire( (LONG *)&overlapped->Internal ); - if (status == STATUS_PENDING) + if (timeout) { - if (!timeout) - { - SetLastError( ERROR_IO_INCOMPLETE ); - return FALSE; - } ret = WaitForSingleObjectEx( overlapped->hEvent ? overlapped->hEvent : file, timeout, alertable ); if (ret == WAIT_FAILED) return FALSE; @@ -3276,15 +3268,23 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file, OVERLAPPED *ov SetLastError( ret ); return FALSE; } - /* We don't need to give this load acquire semantics; the wait above * already guarantees that the IOSB and output buffer are filled. */ status = overlapped->Internal; - if (status == STATUS_PENDING) status = STATUS_SUCCESS; } - + else + { + /* Paired with the write-release in set_async_iosb() in ntdll; see the + * latter for details. */ + if ((status = ReadAcquire( (LONG *)&overlapped->Internal )) == STATUS_PENDING) + { + SetLastError( ERROR_IO_INCOMPLETE ); + return FALSE; + } + } *result = overlapped->InternalHigh; - return set_ntstatus( status ); + SetLastError( RtlNtStatusToDosError( status )); + return !status || status == STATUS_PENDING; }
Turns out GetOverlappedResultEx() is different from GetOverlappedResult() in an important way.
-
Paul Gofman -
Paul Gofman (＠gofman)