handle cases where ReadFile/WriteFile may complete synchronously (#161)

Summary:
Pull Request resolved: #161

Unlike `ReadFileEx` and `WriteFileEx`, which always call their completion routines and are always asynchrous, `ReadFile` and `WriteFile` may complete synchronously. [This MSDN article](https://learn.microsoft.com/en-us/previous-versions/troubleshoot/windows/win32/asynchronous-disk-io-synchronous) discusses how to handle this (which is different from some of the advice given in ReadFile's documentation. That says
> The lpNumberOfBytesRead parameter should be set to NULL. Use the GetOverlappedResult function to get the actual number of bytes read.

If you do that, there's no way to know how many bytes were transferred if the operation completes synchronously.

Reviewed By: georges-berenger

Differential Revision: D61977068

fbshipit-source-id: cb7c4c6e3b86ff4b51ac29e6ec38fd9d382ef14f

vrs/AsyncDiskFileChunk.hpp

@@ -172,30 +172,30 @@ struct AsyncWindowsHandle {
       return readNotWrite ? DISKFILE_NOT_ENOUGH_DATA : DISKFILE_PARTIAL_WRITE_ERROR;
     }
 
-    OVERLAPPED ov;
-
-    ov.hEvent = 0;
+    // N.B. this does not create an hEvent for the OVERLAPPED structure, instead using the file
+    // handle. This is only a valid thing to do if there are NO other IO operations occuring during
+    // this one. The calls to flushWriteBuffer() before calling this ensures this is the case.
+    OVERLAPPED ov = {};
     ov.Offset = (DWORD)offset;
     ov.OffsetHigh = (DWORD)(offset >> 32);
 
+    DWORD dwNumberOfBytesTransferred = 0;
+    bool success = false;
     if (readNotWrite) {
-      if (ReadFile(h_, buf, dwToXfer, nullptr, &ov)) {
-        return SUCCESS;
-      }
+      success = ReadFile(h_, buf, dwToXfer, &dwNumberOfBytesTransferred, &ov);
     } else {
-      if (WriteFile(h_, buf, dwToXfer, nullptr, &ov)) {
-        return SUCCESS;
-      }
+      success = WriteFile(h_, buf, dwToXfer, &dwNumberOfBytesTransferred, &ov);
     }
 
-    int error = GetLastError();
-    if (error != ERROR_IO_PENDING) {
-      return error;
-    }
+    if (!success) {
+      int error = GetLastError();
+      if (error != ERROR_IO_PENDING) {
+        return error;
+      }
 
-    DWORD dwNumberOfBytesTransferred;
-    if (!GetOverlappedResult(h_, &ov, &dwNumberOfBytesTransferred, TRUE)) {
-      return GetLastError();
+      if (!GetOverlappedResult(h_, &ov, &dwNumberOfBytesTransferred, TRUE)) {
+        return GetLastError();
+      }
     }
 
     outSize = dwNumberOfBytesTransferred;

vrs/test/file_tests/DeviceSimulatorTest.cpp

@@ -25,7 +25,6 @@
 
 #include <vrs/DiskFile.h>
 #include <vrs/RecordFileReader.h>
-#include <vrs/os/Platform.h>
 #include <vrs/os/Utils.h>
 
 #include <vrs/test/helpers/VRSTestsHelpers.h>
@@ -128,7 +127,6 @@ TEST_F(DeviceSimulator, multiThreadAsyncAioNotDirect) {
   deleteChunkedFile(testPath);
 }
 
-#if IS_VRS_FB_INTERNAL() || !IS_WINDOWS_PLATFORM() // avoid OSS/Windows
 TEST_F(DeviceSimulator, multiThreadAsyncSync) {
   const string testPath = os::getTempFolder() + "MultiThreadAsyncSync.vrs";
 
@@ -140,7 +138,6 @@ TEST_F(DeviceSimulator, multiThreadAsyncSync) {
 
   deleteChunkedFile(testPath);
 }
-#endif
 
 TEST_F(DeviceSimulator, multiThreadAsyncPsync) {
   const string testPath = os::getTempFolder() + "MultiThreadAsyncPsync.vrs";