file: keep independent Linux file handles coherent

src/common/platform/sysLinuxFileIO.cpp

@@ -152,42 +152,29 @@ void SysFileWrite(const void* data, uint32_t size, sys_file_t& f, uint32_t* byte
 	}
 }
 
-sys_file_t* SysFileCreate(const std::filesystem::path& file_name) {
+static sys_file_t* OpenHostFile(const std::filesystem::path& file_name, const char* mode,
+                               sys_file_cache_type_t cache_type) {
 	auto* ret = new sys_file_t;
-
+	ret->f = fopen(get_internal_name(file_name).c_str(), mode);
-	auto real_name     = get_internal_name(file_name);
+	ret->type = ret->f == nullptr ? SYS_FILE_ERROR : SYS_FILE_FILE;
-	auto real_name_str = real_name.string();
+	if (ret->f != nullptr) {
-
+		// Independent handles must observe writes and truncation immediately.
-	ret->f = fopen(real_name_str.c_str(), "w+");
+		setvbuf(ret->f, nullptr, _IONBF, 0);
-
+		apply_cache_hint(ret->f, cache_type);
-	if (ret->f == nullptr) {
-		printf("can't create file: %s\n", real_name_str.c_str());
 	}
+	return ret;
+}
 
-	ret->type = SYS_FILE_FILE;
+sys_file_t* SysFileCreate(const std::filesystem::path& file_name) {
-
+	auto* ret = OpenHostFile(file_name, "w+", SYS_FILE_CACHE_AUTO);
+	if (ret->f == nullptr) {
+		printf("can't create file: %s\n", file_name.c_str());
+	}
 	return ret;
 }
 
 sys_file_t* SysFileOpenR(const std::filesystem::path& file_name, sys_file_cache_type_t cache_type) {
-	auto* ret = new sys_file_t;
+	return OpenHostFile(file_name, "r", cache_type);
-
-	ret->type = SYS_FILE_FILE;
-
-	auto internal_name     = get_internal_name(file_name);
-	auto internal_name_str = internal_name.string();
-
-	FILE* f = fopen(internal_name_str.c_str(), "r");
-
-	if (f == nullptr) {
-		ret->type = SYS_FILE_ERROR;
-	}
-
-	apply_cache_hint(f, cache_type);
-
-	ret->f = f;
-
-	return ret;
 }
 
 sys_file_t* SysFileOpen(uint8_t* buf, uint32_t buf_size) {
@@ -215,46 +202,12 @@ sys_file_t* SysFileCreate() {
 }
 
 sys_file_t* SysFileOpenW(const std::filesystem::path& file_name, sys_file_cache_type_t cache_type) {
-	auto* ret = new sys_file_t;
+	return OpenHostFile(file_name, "r+", cache_type);
-
-	auto real_name     = get_internal_name(file_name);
-	auto real_name_str = real_name.string();
-
-	FILE* f = fopen(real_name_str.c_str(), "r+");
-
-	if (f == nullptr) {
-		ret->type = SYS_FILE_ERROR;
-	} else {
-		ret->type = SYS_FILE_FILE;
-	}
-
-	apply_cache_hint(f, cache_type);
-
-	ret->f = f;
-
-	return ret;
 }
 
 sys_file_t* SysFileOpenRw(const std::filesystem::path& file_name,
                           sys_file_cache_type_t        cache_type) {
-	auto* ret = new sys_file_t;
+	return OpenHostFile(file_name, "r+", cache_type);
-
-	auto real_name     = get_internal_name(file_name);
-	auto real_name_str = real_name.string();
-
-	FILE* f = fopen(real_name_str.c_str(), "r+");
-
-	if (f == nullptr) {
-		ret->type = SYS_FILE_ERROR;
-	} else {
-		ret->type = SYS_FILE_FILE;
-	}
-
-	apply_cache_hint(f, cache_type);
-
-	ret->f = f;
-
-	return ret;
 }
 
 void SysFileClose(sys_file_t* f) {

tests/KernelFileSystemTests.cpp

@@ -104,6 +104,48 @@ private:
   std::filesystem::path m_path;
 };
 
+void CheckOpenFileCoherence(const std::filesystem::path& root) {
+  const auto path = root / "open-file-coherence.dat";
+  Common::File writer;
+  Check(writer.Create(path), "create shared file fixture");
+  uint32_t count = 0;
+  writer.Write("abcdef", 6, &count);
+  Check(count == 6 && Common::File::Size(path) == 6,
+        "created file writes are visible before flush or close");
+
+  Common::File reader(path, Common::File::Mode::Read);
+  Common::File update(path, Common::File::Mode::Write);
+  Check(!reader.IsInvalid() && !update.IsInvalid(), "open independent file handles");
+  std::array<char, 2> bytes {};
+  reader.Read(bytes.data(), 1, &count);
+  Check(count == 1 && bytes[0] == 'a' && update.Seek(1),
+        "prime sequential read and position independent writer");
+  update.Write("XY", 2, &count);
+  Check(count == 2, "overwrite bytes through write handle");
+  reader.Read(bytes.data(), bytes.size(), &count);
+  Check(count == bytes.size() && std::memcmp(bytes.data(), "XY", bytes.size()) == 0,
+        "sequential reader observes another handle's writes without seeking");
+
+  Common::File read_write(path, Common::File::Mode::ReadWrite);
+  Check(!read_write.IsInvalid() && read_write.Seek(3), "open read-write handle");
+  read_write.Write("Z", 1, &count);
+  Check(count == 1, "overwrite through read-write handle");
+  reader.Read(bytes.data(), 1, &count);
+  Check(count == 1 && bytes[0] == 'Z', "read-write handle changes are immediately visible");
+
+  writer.Write("g", 1, &count);
+  Check(count == 1 && writer.Tell() == 7, "extend through the original writer");
+  std::error_code error;
+  std::filesystem::resize_file(path, 2, error);
+  Check(!error && writer.Tell() == 7 && Common::File::Size(path) == 2,
+        "path resize preserves an open writer's position");
+  writer.Close();
+  update.Close();
+  read_write.Close();
+  Check(Common::File::Size(path) == 2,
+        "closing existing writers cannot undo a path truncation");
+}
+
 void CheckSaveRename(const std::filesystem::path &root,
                      std::string_view payload) {
   constexpr char Source[] = "/savedata0/STEMP000.DAT";
@@ -1851,6 +1893,7 @@ int main(int, char**) {
   graphics->CreateVulkan();
 
   TempDirectory temporary;
+  CheckOpenFileCoherence(temporary.Path());
   FileSystem::Initialize();
   TestSysmoduleReferences();
   TestSystemServiceEntitlementEvents();