- Errors should only be reported to I/O completion routine after

dlls/kernel/tests/file.c

@@ -1208,6 +1208,50 @@ static void test_GetFileType(void)
     DeleteFileA( filename );
 }
 
+static int completion_count;
+
+static void CALLBACK FileIOComplete(DWORD dwError, DWORD dwBytes, LPOVERLAPPED ovl)
+{
+/*	printf("(%ld, %ld, %p { %ld, %ld, %ld, %ld, %p })\n", dwError, dwBytes, ovl, ovl->Internal, ovl->InternalHigh, ovl->Offset, ovl->OffsetHigh, ovl->hEvent);*/
+	ReleaseSemaphore(ovl->hEvent, 1, NULL);
+	completion_count++;
+}
+
+static void test_async_file_errors(void)
+{
+    char szFile[MAX_PATH];
+    HANDLE hSem = CreateSemaphoreW(NULL, 1, 1, NULL);
+    HANDLE hFile;
+    LPVOID lpBuffer = HeapAlloc(GetProcessHeap(), 0, 4096);
+    OVERLAPPED ovl;
+    ovl.Offset = 0;
+    ovl.OffsetHigh = 0;
+    ovl.hEvent = hSem;
+    completion_count = 0;
+    szFile[0] = '\0';
+    GetWindowsDirectoryA(szFile, sizeof(szFile)/sizeof(szFile[0])-1-strlen("\\win.ini"));
+    strcat(szFile, "\\win.ini");
+    hFile = CreateFileA(szFile, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_ALWAYS, FILE_FLAG_OVERLAPPED, NULL);
+    ok(hFile != NULL, "CreateFileA(%s ...) failed\n", szFile);
+    while (TRUE)
+    {
+        BOOL res;
+        while (WaitForSingleObjectEx(hSem, INFINITE, TRUE) == WAIT_IO_COMPLETION)
+            ;
+        res = ReadFileEx(hFile, lpBuffer, 4096, &ovl, FileIOComplete);
+        /*printf("Offset = %ld, result = %s\n", ovl.Offset, res ? "TRUE" : "FALSE");*/
+        if (!res)
+            break;
+        ovl.Offset += 4096;
+        /* i/o completion routine only called if ReadFileEx returned success.
+         * we only care about violations of this rule so undo what should have
+         * been done */
+        completion_count--;
+    }
+    ok(completion_count == 0, "completion routine should only be called when ReadFileEx succeeds (this rule was violated %d times)\n", completion_count);
+    /*printf("Error = %ld\n", GetLastError());*/
+}
+
 START_TEST(file)
 {
     test__hread(  );
@@ -1234,4 +1278,5 @@ START_TEST(file)
     test_offset_in_overlapped_structure();
     test_MapFile();
     test_GetFileType();
+    test_async_file_errors();
 }

dlls/ntdll/file.c

@@ -248,6 +248,7 @@ typedef struct async_fileio
     char                             *buffer;
     unsigned int                     count;
     off_t                            offset;
+    int                              queue_apc_on_error;
     BOOL                             avail_mode;
 } async_fileio;
 
@@ -265,7 +266,8 @@ static void CALLBACK fileio_call_completion_func(ULONG_PTR data)
     async_fileio *ovp = (async_fileio*) data;
     TRACE("data: %p\n", ovp);
 
-    ovp->apc( ovp->apc_user, ovp->async.iosb, ovp->async.iosb->Information );
+    if ((ovp->async.iosb->u.Status == STATUS_SUCCESS) || ovp->queue_apc_on_error)
+        ovp->apc( ovp->apc_user, ovp->async.iosb, ovp->async.iosb->Information );
 
     fileio_async_cleanup( &ovp->async );
 }
@@ -464,6 +466,7 @@ NTSTATUS WINAPI NtReadFile(HANDLE hFile, HANDLE hEvent,
         ovp->apc = apc;
         ovp->apc_user = apc_user;
         ovp->buffer = buffer;
+        ovp->queue_apc_on_error = 0;
         ovp->avail_mode = (flags & FD_FLAG_AVAILABLE);
         NtResetEvent(hEvent, NULL);
 
@@ -477,8 +480,10 @@ NTSTATUS WINAPI NtReadFile(HANDLE hFile, HANDLE hEvent,
         }
         if (flags & FD_FLAG_TIMEOUT)
         {
-            NtWaitForSingleObject(hEvent, TRUE, NULL);
+            ret = NtWaitForSingleObject(hEvent, TRUE, NULL);
             NtClose(hEvent);
+            if (ret != STATUS_USER_APC)
+                ovp->queue_apc_on_error = 1;
         }
         else
         {
@@ -486,7 +491,14 @@ NTSTATUS WINAPI NtReadFile(HANDLE hFile, HANDLE hEvent,
 
             /* let some APC be run, this will read some already pending data */
             timeout.u.LowPart = timeout.u.HighPart = 0;
-            NtDelayExecution( TRUE, &timeout );
+            ret = NtDelayExecution( TRUE, &timeout );
+            /* the apc didn't run and therefore the completion routine now
+             * needs to be sent errors.
+             * Note that there is no race between setting this flag and
+             * returning errors because apc's are run only during alertable
+             * waits */
+            if (ret != STATUS_USER_APC)
+                ovp->queue_apc_on_error = 1;
             /* if we only have to read the available data, and none is available,
              * simply cancel the request. If data was available, it has been read
              * while in by previous call (NtDelayExecution)
@@ -497,6 +509,7 @@ NTSTATUS WINAPI NtReadFile(HANDLE hFile, HANDLE hEvent,
                 register_old_async(&ovp->async);
             }
         }
+        TRACE("= 0x%08lx\n", io_status->u.Status);
         return io_status->u.Status;
     }
 
@@ -522,6 +535,7 @@ NTSTATUS WINAPI NtReadFile(HANDLE hFile, HANDLE hEvent,
 	break;
     }
     wine_server_release_fd( hFile, unix_handle );
+    TRACE("= 0x%08lx\n", io_status->u.Status);
     return io_status->u.Status;
 }
 
@@ -641,6 +655,7 @@ NTSTATUS WINAPI NtWriteFile(HANDLE hFile, HANDLE hEvent,
         ovp->apc = apc;
         ovp->apc_user = apc_user;
         ovp->buffer = (void*)buffer;
+        ovp->queue_apc_on_error = 0;
         ovp->avail_mode = (flags & FD_FLAG_AVAILABLE);
         NtResetEvent(hEvent, NULL);
 
@@ -650,8 +665,10 @@ NTSTATUS WINAPI NtWriteFile(HANDLE hFile, HANDLE hEvent,
             return ret;
         if (flags & FD_FLAG_TIMEOUT)
         {
-            NtWaitForSingleObject(hEvent, TRUE, NULL);
+            ret = NtWaitForSingleObject(hEvent, TRUE, NULL);
             NtClose(hEvent);
+            if (ret != STATUS_USER_APC)
+                ovp->queue_apc_on_error = 1;
         }
         else
         {
@@ -659,7 +676,14 @@ NTSTATUS WINAPI NtWriteFile(HANDLE hFile, HANDLE hEvent,
 
             /* let some APC be run, this will write as much data as possible */
             timeout.u.LowPart = timeout.u.HighPart = 0;
-            NtDelayExecution( TRUE, &timeout );
+            ret = NtDelayExecution( TRUE, &timeout );
+            /* the apc didn't run and therefore the completion routine now
+             * needs to be sent errors.
+             * Note that there is no race between setting this flag and
+             * returning errors because apc's are run only during alertable
+             * waits */
+            if (ret != STATUS_USER_APC)
+                ovp->queue_apc_on_error = 1;
         }
         return io_status->u.Status;
     }