IOS/FS: Fix loading savestate when files are open

We already had code to close open host files when reading or writing a
savestate, but due to d35fe1b we also need to close open guest files
when reading a savestate, otherwise DoStateRead fails to delete them.

I was considering an alternative solution where instead of copying and
clearing m_handles, we just set `handle.opened = false;` for each handle
before reading a savestate (but not before writing a savestate).
However, this wouldn't solve the problem of DoStateWriteOrMeasure's
calls to OpenFile failing due to all handles being open. I'm not aware
of any games that have that many handles open, though.

Source/Core/Core/IOS/FS/HostBackend/FS.cpp

@@ -279,8 +279,18 @@ HostFileSystem::FstEntry* HostFileSystem::GetFstEntryForPath(const std::string&
 
 void HostFileSystem::DoState(PointerWrap& p)
 {
-  // Temporarily close the file, to prevent any issues with the savestating of files/folders.
-  for (Handle& handle : m_handles)
+  // This piece of code is handling four separate problems:
+  // 1. Close host handles by calling reset on them, in case DoStateRead needs to modify a file that
+  //    was open.
+  // 2. Close guest handles by setting opened to false on each element in m_handles, in case
+  //    DoStateRead needs to modify a file that was open.
+  // 3. Close guest handles by setting opened to false on each element in m_handles, because if all
+  //    of them were open, it would make DoStateRead/DoStateWriteOrMeasure's calls to OpenFile fail.
+  // 4. Create a copy of m_handles that we can restore later in case we're writing/measuring,
+  //    because OpenFile happily stomps over elements in m_handles that have opened set to false.
+  auto handles_copy = std::move(m_handles);
+  m_handles = {};
+  for (Handle& handle : handles_copy)
     handle.host_file.reset();
 
   // The format for the next part of the save state is follows:
@@ -316,6 +326,8 @@ void HostFileSystem::DoState(PointerWrap& p)
         memcpy(nand_size_ptr, &size_of_nand, sizeof(size_of_nand));
       }
     }
+
+    m_handles = std::move(handles_copy);
   }
   else  // case where we're in read mode.
   {

Source/UnitTests/Core/IOS/FS/FileSystemTest.cpp

@@ -9,6 +9,7 @@
 
 #include <gtest/gtest.h>
 
+#include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
 #include "Common/FileUtil.h"
 #include "Core/IOS/FS/FileSystem.h"
@@ -469,3 +470,108 @@ TEST_F(FileSystemTest, CreateFullPath)
   EXPECT_EQ(m_fs->CreateFullPath(Uid{0x1000}, Gid{1}, "/shared2/wc24/mbox/Readme.txt", 0, modes),
             ResultCode::Success);
 }
+
+TEST_F(FileSystemTest, DoState)
+{
+  const std::string TEST_DATA_1 = "123";
+  const std::string TEST_DATA_2 = "4567";
+
+  std::array<u8, 4> read_buffer;
+
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{1}, Gid{2}, "/tmp/a", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{1}, Gid{2}, "/tmp/a/b", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{0}, Gid{0}, "/tmp/a/c", 0, modes), ResultCode::Success);
+
+  ASSERT_EQ(m_fs->CreateFile(Uid{1}, Gid{2}, "/tmp/a/d", 0, modes), ResultCode::Success);
+  ASSERT_EQ(m_fs->CreateFile(Uid{3}, Gid{4}, "/tmp/e", 0, modes), ResultCode::Success);
+
+  {
+    Result<FileHandle> file1 = m_fs->OpenFile(Uid{1}, Gid{2}, "/tmp/a/d", Mode::ReadWrite);
+    ASSERT_TRUE(file1.has_value());
+    ASSERT_TRUE(file1->Write(TEST_DATA_1.data(), TEST_DATA_1.size()).has_value());
+  }
+
+  std::array<u8, 1024> state_buffer;
+  size_t state_size;
+
+  {
+    Result<FileHandle> file2 = m_fs->OpenFile(Uid{3}, Gid{4}, "/tmp/e", Mode::ReadWrite);
+    ASSERT_TRUE(file2.has_value());
+    ASSERT_TRUE(file2->Write(TEST_DATA_2.data(), TEST_DATA_2.size()).has_value());
+
+    u8* state_pointer = state_buffer.data();
+    PointerWrap p(&state_pointer, state_buffer.size(), PointerWrap::Mode::Write);
+    m_fs->DoState(p);
+    ASSERT_TRUE(p.IsWriteMode());
+
+    ASSERT_TRUE(file2->Seek(2, SeekMode::Set).has_value());
+    ASSERT_TRUE(file2->Write("_", 1).has_value());
+
+    Fd fd = file2->Release();
+    p.Do(fd);
+    ASSERT_TRUE(p.IsWriteMode());
+
+    state_size = state_pointer - state_buffer.data();
+  }
+
+  ASSERT_EQ(m_fs->Delete(Uid{0}, Gid{0}, "/tmp/a"), ResultCode::Success);
+  ASSERT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a").error(), ResultCode::NotFound);
+  ASSERT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a/b").error(), ResultCode::NotFound);
+  ASSERT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a/c").error(), ResultCode::NotFound);
+  ASSERT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a/d").error(), ResultCode::NotFound);
+
+  ASSERT_EQ(m_fs->CreateFile(Uid{5}, Gid{6}, "/tmp/f", 0, modes), ResultCode::Success);
+
+  ASSERT_EQ(m_fs->CreateDirectory(Uid{7}, Gid{8}, "/tmp/g", 0, modes), ResultCode::Success);
+
+  u8* state_pointer = state_buffer.data();
+  PointerWrap p(&state_pointer, state_size, PointerWrap::Mode::Read);
+  m_fs->DoState(p);
+  ASSERT_TRUE(p.IsReadMode());
+
+  constexpr auto check_directory_metadata = [](const Result<Metadata>& metadata, Uid uid, Gid gid) {
+    ASSERT_TRUE(metadata.has_value());
+    ASSERT_EQ(metadata->uid, uid);
+    ASSERT_EQ(metadata->gid, gid);
+    ASSERT_FALSE(metadata->is_file);
+  };
+
+  constexpr auto check_file_metadata = [](const Result<Metadata>& metadata, Uid uid, Gid gid,
+                                          u32 size) {
+    ASSERT_TRUE(metadata.has_value());
+    ASSERT_EQ(metadata->uid, uid);
+    ASSERT_EQ(metadata->gid, gid);
+    ASSERT_TRUE(metadata->is_file);
+    ASSERT_EQ(metadata->size, size);
+  };
+
+  check_directory_metadata(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a"), Uid{1}, Gid{2});
+  check_directory_metadata(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a/b"), Uid{1}, Gid{2});
+  check_directory_metadata(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a/c"), Uid{0}, Gid{0});
+
+  check_file_metadata(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/a/d"), Uid{1}, Gid{2}, 3);
+  check_file_metadata(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/e"), Uid{3}, Gid{4}, 4);
+
+  ASSERT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/f").error(), ResultCode::NotFound);
+  ASSERT_EQ(m_fs->GetMetadata(Uid{0}, Gid{0}, "/tmp/g").error(), ResultCode::NotFound);
+
+  Fd fd{};
+  p.Do(fd);
+  ASSERT_TRUE(p.IsReadMode());
+
+  {
+    FileHandle file2(m_fs.get(), fd);
+    ASSERT_EQ(file2.GetStatus()->offset, 4u);
+    ASSERT_TRUE(file2.Seek(0, SeekMode::Set).has_value());
+    ASSERT_TRUE(file2.Read(read_buffer.data(), TEST_DATA_2.size()).has_value());
+    for (size_t i = 0; i < TEST_DATA_2.size(); ++i)
+      ASSERT_EQ(read_buffer[i], TEST_DATA_2[i]);
+  }
+
+  {
+    Result<FileHandle> file1 = m_fs->OpenFile(Uid{5}, Gid{6}, "/tmp/a/d", Mode::Read);
+    ASSERT_TRUE(file1->Read(read_buffer.data(), TEST_DATA_1.size()).has_value());
+    for (size_t i = 0; i < TEST_DATA_1.size(); ++i)
+      ASSERT_EQ(read_buffer[i], TEST_DATA_1[i]);
+  }
+}