Support large files in FileReader on Windows (#1490)

Muon · web-flow · commit a7eeff1383cc · 2025-11-25T13:47:48.000+11:00
### Changelog * C++: Support reading files of size 2GiB+ using `FileReader` on Windows. ### Docs None ### Description `fseek` and `ftell` use `long` for offsets, which is a signed 32-bit integer even on 64-bit Windows. This limited `FileReader` to files of size 2^31 - 1. When getting the file's size, larger files would cause `ftell` to return -1 (indicating an error). The return value was not checked, and this was cast to `uint64_t`, wrapping the value to 2^64 - 1. This PR changes the implementation to instead use the Windows-specific `_ftelli64` and `_fseeki64` which take an `__int64` instead, and also to check the return values. This PR also means that 32-bit POSIX platforms will now cause the `FileReader` constructor to throw an exception when passed a file of size 2GiB+ rather than running into the same problem. I have added a `FileReader` unit test generating a large sparse file and checked that it fails without the change to `FileReader`. Fixes #1486. Fixes: FIRE-221
diff --git a/cpp/bench/conanfile.py b/cpp/bench/conanfile.py
@@ -4,7 +4,7 @@
 class McapBenchmarksConan(ConanFile):
     settings = "os", "compiler", "build_type", "arch"
     generators = "cmake"
-    requires = "benchmark/1.7.0", "mcap/2.1.1"
+    requires = "benchmark/1.7.0", "mcap/2.1.2"
 
     def build(self):
         cmake = CMake(self)
diff --git a/cpp/build-docs.sh b/cpp/build-docs.sh
@@ -4,7 +4,7 @@ set -e
 
 conan config init
 
-conan editable add ./mcap mcap/2.1.1
+conan editable add ./mcap mcap/2.1.2
 conan install docs --install-folder docs/build/Release \
   -s compiler.cppstd=17 -s build_type=Release --build missing
 
diff --git a/cpp/build.sh b/cpp/build.sh
@@ -4,7 +4,7 @@ set -e
 
 conan config init
 
-conan editable add ./mcap mcap/2.1.1
+conan editable add ./mcap mcap/2.1.2
 conan install test --install-folder test/build/Debug \
   -s compiler.cppstd=17 -s build_type=Debug --build missing
 
diff --git a/cpp/docs/conanfile.py b/cpp/docs/conanfile.py
@@ -4,7 +4,7 @@
 class McapDocsConan(ConanFile):
     settings = "os", "compiler", "build_type", "arch"
     generators = "cmake"
-    requires = "mcap/2.1.1"
+    requires = "mcap/2.1.2"
 
     def build(self):
         cmake = CMake(self)
diff --git a/cpp/examples/conanfile.py b/cpp/examples/conanfile.py
@@ -5,7 +5,7 @@ class McapExamplesConan(ConanFile):
     settings = "os", "compiler", "build_type", "arch"
     generators = "cmake"
     requires = [
-        "mcap/2.1.1",
+        "mcap/2.1.2",
         "protobuf/3.21.1",
         "nlohmann_json/3.10.5",
         "catch2/2.13.8",
diff --git a/cpp/mcap/conanfile.py b/cpp/mcap/conanfile.py
@@ -3,7 +3,7 @@
 
 class McapConan(ConanFile):
     name = "mcap"
-    version = "2.1.1"
+    version = "2.1.2"
     url = "https://github.com/foxglove/mcap"
     homepage = "https://github.com/foxglove/mcap"
     description = "A C++ implementation of the MCAP file format"
diff --git a/cpp/mcap/include/mcap/reader.hpp b/cpp/mcap/include/mcap/reader.hpp
@@ -80,6 +80,18 @@ class MCAP_PUBLIC FileReader final : public IReadable {
   uint64_t read(std::byte** output, uint64_t offset, uint64_t size) override;
 
 private:
+  // Numeric type returned by the tell/seek operations. Necessary because long on Windows is 32
+  // bits so the standard C library interfaces don't work for files larger than 2GiB.
+#if defined _WIN32 || defined __CYGWIN__
+  typedef __int64 offset_type;
+#else
+  typedef long offset_type;
+#endif
+
+  static_assert((offset_type)(uint64_t)std::numeric_limits<offset_type>::max() ==
+                  std::numeric_limits<offset_type>::max(),
+                "offset_type should fit in uint64_t");
+
   std::FILE* file_;
   std::vector<std::byte> buffer_;
   uint64_t size_;
diff --git a/cpp/mcap/include/mcap/reader.inl b/cpp/mcap/include/mcap/reader.inl
@@ -51,9 +51,21 @@ FileReader::FileReader(std::FILE* file)
   assert(file_);
 
   // Determine the size of the file
-  std::fseek(file_, 0, SEEK_END);
-  size_ = std::ftell(file_);
-  std::fseek(file_, 0, SEEK_SET);
+  if (std::fseek(file_, 0, SEEK_END) != 0) {
+    throw std::system_error(errno, std::generic_category());
+  }
+#if defined _WIN32 || defined __CYGWIN__
+  offset_type s = _ftelli64(file_);
+#else
+  offset_type s = std::ftell(file_);
+#endif
+  if (s < 0) {
+    throw std::system_error(errno, std::generic_category());
+  }
+  size_ = s;
+  if (std::fseek(file_, 0, SEEK_SET) != 0) {
+    throw std::system_error(errno, std::generic_category());
+  }
 }
 
 uint64_t FileReader::size() const {
@@ -64,20 +76,36 @@ uint64_t FileReader::read(std::byte** output, uint64_t offset, uint64_t size) {
   if (offset >= size_) {
     return 0;
   }
+  // Clamp read size to SIZE_MAX. This should only be relevant on 32-bit platforms. If clamping
+  // does happen, allocation will fail, but this ensures the error is surfaced sooner. Otherwise,
+  // wraparound will produce the wrong read size, e.g. a read of 2^32 becomes a read of 0.
+  // We should really be taking a size_t instead and shifting the responsibility to the caller.
+  const std::size_t read_size = size <= (uint64_t)SIZE_MAX ? size : SIZE_MAX;
 
   if (offset != position_) {
-    std::fseek(file_, (long)(offset), SEEK_SET);
     std::fflush(file_);
+    // offset <= size_ and size_ fits into offset_type by definition so these conversions are
+    // lossless.
+#if defined _WIN32 || defined __CYGWIN__
+    int status = _fseeki64(file_, (offset_type)offset, SEEK_SET);
+#else
+    int status = std::fseek(file_, (offset_type)offset, SEEK_SET);
+#endif
+    if (status < 0) {
+      throw std::system_error(errno, std::generic_category());
+    }
     position_ = offset;
   }
 
-  if (size > buffer_.size()) {
-    buffer_.resize(size);
+  if (read_size > buffer_.size()) {
+    buffer_.resize(read_size);
   }
 
-  const uint64_t bytesRead = uint64_t(std::fread(buffer_.data(), 1, size, file_));
+  const uint64_t bytesRead = uint64_t(std::fread(buffer_.data(), 1, read_size, file_));
   *output = buffer_.data();
 
+  // This should not overflow unless the file size exceeds 2^64 - 1 bytes, which should be
+  // impossible even if the file grows while we're reading it.
   position_ += bytesRead;
   return bytesRead;
 }
diff --git a/cpp/mcap/include/mcap/types.hpp b/cpp/mcap/include/mcap/types.hpp
@@ -14,7 +14,7 @@
 
 namespace mcap {
 
-#define MCAP_LIBRARY_VERSION "2.1.1"
+#define MCAP_LIBRARY_VERSION "2.1.2"
 
 using SchemaId = uint16_t;
 using ChannelId = uint16_t;
diff --git a/cpp/test/conanfile.py b/cpp/test/conanfile.py
@@ -4,7 +4,7 @@
 class McapTestConan(ConanFile):
     settings = "os", "compiler", "build_type", "arch"
     generators = "cmake"
-    requires = "catch2/2.13.8", "mcap/2.1.1", "nlohmann_json/3.10.5"
+    requires = "catch2/2.13.8", "mcap/2.1.2", "nlohmann_json/3.10.5"
 
     def build(self):
         cmake = CMake(self)
diff --git a/cpp/test/unit_tests.cpp b/cpp/test/unit_tests.cpp
@@ -5,8 +5,15 @@
 #include <catch2/catch.hpp>
 
 #include <array>
+#include <cstdio>
 #include <numeric>
 
+#if defined _WIN32 || defined __CYGWIN__
+#  include <io.h>
+#  include <ioapiset.h>
+#  include <winioctl.h>
+#endif
+
 std::string_view StringView(const std::byte* data, size_t size) {
   return std::string_view{reinterpret_cast<const char*>(data), size};
 }
@@ -1129,3 +1136,30 @@ TEST_CASE("Schema isolation between files with noRepeatedSchemas=false", "[write
     reader.close();
   }
 }
+
+TEST_CASE("FileReader works on files larger than 2GiB") {
+  std::FILE* file = std::tmpfile();
+#if defined _WIN32 || defined __CYGWIN__
+  // Seeking past the end automatically makes sparse files on POSIX platforms if the filesystem
+  // supports it, but Windows needs an ioctl. Without this, the test runs much slower.
+  {
+    HANDLE hFile = (HANDLE)_get_osfhandle(_fileno(file));
+    REQUIRE(hFile != INVALID_HANDLE_VALUE);
+    REQUIRE(DeviceIoControl(hFile, FSCTL_SET_SPARSE, nullptr, 0, nullptr, 0, nullptr, nullptr) ==
+            TRUE);
+  }
+#endif
+  // 2^30 + 2^30 = 2^31 > 2^31 - 1
+  REQUIRE(std::fseek(file, 1L << 30L, SEEK_CUR) == 0);
+  REQUIRE(std::fseek(file, 1L << 30L, SEEK_CUR) == 0);
+  REQUIRE(std::fwrite("X", 1, 1, file) == 1);
+  REQUIRE(std::ferror(file) == 0);
+  std::rewind(file);
+  auto reader = mcap::FileReader(file);
+  REQUIRE(reader.size() == (1LL << 31LL) + 1);
+  std::byte* output;
+  REQUIRE(reader.read(&output, 1LL << 31LL, 1) == 1);
+  REQUIRE((char)*output == 'X');
+  REQUIRE(std::ferror(file) == 0);
+  std::fclose(file);
+}
diff --git a/cspell.config.yaml b/cspell.config.yaml
@@ -145,6 +145,11 @@ overrides:
       - cppstd
       - hdoc
       - nlohmann
+      # Windows-specific functions and includes
+      - ftelli
+      - fseeki
+      - winioctl
+      - ioapiset
     ignoreRegExpList:
       - "LZ4F_\\w+"
       - "LZ4HC_\\w+"
