Fix IOProxy::pread on Windows that botched PNG & JPEG read (#2231)

We've been plagued by recently-introduced bugs with PNG and JPEG
reading that stemmed from the first IOProxy-related patch.  The
problem was our use of the Windows ReadFile call, which seems to not
preserve the file pointer as we'd assumed (the MSDN docs are ambiguous
about this). I also began to doubt its thread safety, so in the end I
just replace the code with a simpler approach with a mutex and calls
to tell/seek/read.

Same changes to pwrite implementation.

CHANGES.md

@@ -1,5 +1,6 @@
-Release 2.0.8 (1 May, 2019) -- compared to 2.0.7
+Release 2.0.8 (3 May, 2019) -- compared to 2.0.7
 ------------------------------------------------
+* Fix Windows broken read of JPEG & PNG in some circumstances. #2231
 * Some minor fixes to JPEG & PNG reading and file error robustness. #2187
 * Fix crash reading certain old nconvert-written TIFF files. #2207
 * Internals: The `OIIO_DISPATCH_COMMON_TYPES2/3` macros used by many

src/include/OpenImageIO/filesystem.h

@@ -420,6 +420,7 @@ class OIIO_API IOFile : public IOProxy {
     FILE* m_file      = nullptr;
     size_t m_size     = 0;
     bool m_auto_close = false;
+    std::mutex m_mutex;
 };
 
 

src/libutil/filesystem.cpp

@@ -988,16 +988,12 @@ Filesystem::IOFile::pread(void* buf, size_t size, int64_t offset)
     if (!m_file || !size || offset < 0 || m_mode != Read)
         return 0;
 #ifdef _WIN32
-    // See MSDN ReadFile docs:
-    // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365467(v=vs.85).aspx
-    HANDLE h = HANDLE(_get_osfhandle(_fileno(m_file)));
-    OVERLAPPED overlapped;
-    memset(&overlapped, 0, sizeof(overlapped));
-    overlapped.Offset     = DWORD(offset);
-    overlapped.OffsetHigh = DWORD(offset >> 32);
-    DWORD bytesread       = 0;
-    return ReadFile(h, buf, DWORD(size), &bytesread, &overlapped) ? bytesread
-                                                                  : 0;
+    std::lock_guard<std::mutex> lock(m_mutex);
+    auto origpos = tell();
+    seek(offset);
+    size_t r = read(buf, size);
+    seek(origpos);
+    return r;
 #else /* Non-Windows: assume POSIX pread is available */
     int fd = fileno(m_file);
     return ::pread(fd, buf, size, offset);
@@ -1022,17 +1018,12 @@ Filesystem::IOFile::pwrite(const void* buf, size_t size, int64_t offset)
     if (!m_file || !size || offset < 0 || m_mode != Write)
         return 0;
 #ifdef _WIN32
-    // See MSDN WriteFile docs:
-    // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365747(v=vs.85).aspx
-    HANDLE h = HANDLE(_get_osfhandle(_fileno(m_file)));
-    OVERLAPPED overlapped;
-    memset(&overlapped, 0, sizeof(overlapped));
-    overlapped.Offset     = DWORD(offset);
-    overlapped.OffsetHigh = DWORD(offset >> 32);
-    DWORD byteswritten    = 0;
-    size_t r = WriteFile(h, buf, DWORD(size), &byteswritten, &overlapped)
-                   ? byteswritten
-                   : 0;
+    std::lock_guard<std::mutex> lock(m_mutex);
+    auto origpos = tell();
+    seek(offset);
+    size_t r = write(buf, size);
+    seek(origpos);
+    return r;
 #else /* Non-Windows: assume POSIX pwrite is available */
     int fd   = fileno(m_file);
     size_t r = ::pwrite(fd, buf, size, offset);