Merge pull request #771 from michaelm-afk/pr/seamless-scoped-disc-reader

Reuse the effective disc reader during resident pack preparation

runtime/CMakeLists.txt

@@ -1589,6 +1589,14 @@ if(BUILD_TESTING)
     endif()
     add_test(NAME mod_resident_test COMMAND mod_resident_test)
 
+    add_executable(mod_disc_read_session_test tests/test_mod_disc_read_session.cpp
+        src/iso_reader.cpp src/cue_sheet.cpp)
+    target_include_directories(mod_disc_read_session_test PRIVATE include ../recompiler/include)
+    target_compile_features(mod_disc_read_session_test PRIVATE cxx_std_17)
+    target_link_libraries(mod_disc_read_session_test PRIVATE chdr-static)
+    add_test(NAME mod_disc_read_session_test COMMAND mod_disc_read_session_test
+        "${CMAKE_CURRENT_BINARY_DIR}/disc-read-session-fixture")
+
     if(WIN32)
         add_executable(autocompile_child_env_test tests/test_autocompile_child_env.c)
         target_include_directories(autocompile_child_env_test PRIVATE src)

runtime/include/mod_disc_read_session.h

@@ -0,0 +1,43 @@
+#pragma once
+#include "iso_reader.h"
+#include <memory>
+#include <string>
+
+namespace PSXRecompV4 {
+/* Preparation-local host IO only. No file extents or patched sectors are cached.
+ * A fresh scope opens the effective disc again; nested scopes share its handle.
+ * Directory lookups still read the current effective sector overlay. */
+struct ModDiscReadSessionState {
+    unsigned depth = 0;
+    std::string mount;
+    std::unique_ptr<PS1::ISOReader> reader;
+};
+inline ModDiscReadSessionState& mod_disc_read_session_state() {
+    static thread_local ModDiscReadSessionState state;
+    return state;
+}
+class ModDiscReadSession {
+public:
+    ModDiscReadSession() { ++mod_disc_read_session_state().depth; }
+    ~ModDiscReadSession() {
+        auto& s = mod_disc_read_session_state();
+        if (!--s.depth) { s.reader.reset(); s.mount.clear(); }
+    }
+    ModDiscReadSession(const ModDiscReadSession&) = delete;
+    ModDiscReadSession& operator=(const ModDiscReadSession&) = delete;
+};
+/* Outside the bounded prepare scope, retain the caller's original fresh reader.
+ * A changed mount invalidates the shared reader. Failed opens are never cached. */
+inline PS1::ISOReader* mod_disc_read_reader(const std::string& mount,
+                                           PS1::ISOReader& fresh) {
+    auto& s = mod_disc_read_session_state();
+    if (!s.depth) return fresh.Open(mount) ? &fresh : nullptr;
+    if (!s.reader || s.mount != mount) {
+        s.reader.reset(); s.mount.clear();
+        auto next = std::make_unique<PS1::ISOReader>();
+        if (!next->Open(mount)) return nullptr;
+        s.mount = mount; s.reader = std::move(next);
+    }
+    return s.reader.get();
+}
+} // namespace PSXRecompV4

runtime/src/mod_resident.cpp

@@ -11,6 +11,7 @@
 #include "mod_resident.h"
 #include "mod_plugins.h"
 #include "mod_runtime.h"
+#include "mod_disc_read_session.h"
 #include "psx_sha256.h"
 #include "host_sha256.h"
 
@@ -397,6 +398,7 @@ extern "C" const PSXResidentPack* psx_resident_prepare(const PSXResidentSpec* sp
         psx_mod_counter_add("resident.invalid_spec", 1);
         return nullptr;
     }
+    PSXRecompV4::ModDiscReadSession disc_reads;
     Record& rec = record_for(spec);
     rec.pack.reset();
     rec.error.clear();

runtime/src/mod_runtime.cpp

@@ -9,6 +9,7 @@
 #include "func_override.h"
 #include "host_sha256.h"
 #include "iso_reader.h"
+#include "mod_disc_read_session.h"
 #include "mod_packages.h"
 #include "mod_plugins.h"
 #include "mod_netplay.h"
@@ -3063,11 +3064,13 @@ extern "C" int psx_mod_read_disc_file(const char* path, void* buffer,
         const auto& s = state();
         const auto mount = mounted_disc_path(s);
         if (mount.empty()) return 0;
-        PS1::ISOReader reader;
+        PS1::ISOReader fresh;
+        PS1::ISOReader* shared = mod_disc_read_reader(mount.string(), fresh);
+        if (!shared) return 0;
+        PS1::ISOReader& reader = *shared;
         uint32_t lba = 0, bytes = 0;
         bool directory = false;
-        if (!reader.Open(mount.string()) ||
-            !find_effective_file(reader, path, lba, bytes, directory) ||
+        if (!find_effective_file(reader, path, lba, bytes, directory) ||
             directory || !bytes || bytes > kMaxDiscFileBytes)
             return 0;
         if (!buffer) { *size = bytes; return 1; }
@@ -3098,11 +3101,13 @@ extern "C" int psx_mod_disc_file_extent(const char* path, uint32_t* lba,
         const auto& s = state();
         const auto mount = mounted_disc_path(s);
         if (mount.empty()) return 0;
-        PS1::ISOReader reader;
+        PS1::ISOReader fresh;
+        PS1::ISOReader* shared = mod_disc_read_reader(mount.string(), fresh);
+        if (!shared) return 0;
+        PS1::ISOReader& reader = *shared;
         uint32_t first = 0, bytes = 0;
         bool directory = false;
-        if (!reader.Open(mount.string()) ||
-            !find_effective_file(reader, path, first, bytes, directory) ||
+        if (!find_effective_file(reader, path, first, bytes, directory) ||
             directory || !bytes || bytes > kMaxDiscFileBytes)
             return 0;
         *lba = first;
@@ -3128,9 +3133,11 @@ bool mod_runtime_read_disc_file_sectors(const std::string& path, uint32_t max_by
         const auto& s = state();
         const auto mount = mounted_disc_path(s);
         if (mount.empty()) return fail("no disc mounted");
-        PS1::ISOReader reader;
+        PS1::ISOReader fresh;
+        PS1::ISOReader* shared = mod_disc_read_reader(mount.string(), fresh);
+        if (!shared) return fail("cannot open disc");
+        PS1::ISOReader& reader = *shared;
         bool directory = false;
-        if (!reader.Open(mount.string())) return fail("cannot open disc");
         if (!find_effective_file(reader, path, lba, size, directory) || directory)
             return fail("not found on the disc");
         if (!size || size > kMaxDiscFileBytes || (max_bytes && size > max_bytes))

runtime/tests/test_mod_disc_read_session.cpp

@@ -0,0 +1,56 @@
+/* Host-only scoped-reader regression; generated empty CUE/BIN fixture, no game data. */
+#ifdef NDEBUG
+#undef NDEBUG
+#endif
+#include "mod_disc_read_session.h"
+#include <cassert>
+#include <cstdio>
+#include <stdexcept>
+#include <filesystem>
+#include <fstream>
+#include <chrono>
+#include <vector>
+using namespace PSXRecompV4;
+int main(int argc,char**argv) {
+    assert(argc==2);
+    const auto dir=std::filesystem::path(argv[1])/std::to_string(
+        std::chrono::steady_clock::now().time_since_epoch().count());
+    assert(!std::filesystem::exists(dir));std::filesystem::create_directories(dir);
+    /* Declared before the readers, so their handles close before removal on
+     * Windows. Each successful run owns and removes only its unique fixture. */
+    struct FixtureCleanup {
+        std::filesystem::path dir;
+        ~FixtureCleanup() {
+            std::error_code ec;
+            std::filesystem::remove_all(dir,ec);
+            assert(!ec);
+        }
+    } cleanup{dir};
+    {std::ofstream out(dir/"data.bin",std::ios::binary);std::vector<char> raw(2352*24,0);out.write(raw.data(),raw.size());}
+    for(const char* name:{"first.cue","second.cue"}){
+        std::ofstream out(dir/name);out<<"FILE \"data.bin\" BINARY\n TRACK 01 MODE2/2352\n INDEX 01 00:00:00\n";
+    }
+    const std::string first_path=(dir/"first.cue").string(),second_path=(dir/"second.cue").string();
+    auto& state=mod_disc_read_session_state();
+    PS1::ISOReader fresh;
+    assert(!state.depth && !state.reader);
+    assert(mod_disc_read_reader(first_path,fresh)==&fresh);
+    {
+        ModDiscReadSession outer;
+        PS1::ISOReader a,b,c;
+        auto* first=mod_disc_read_reader(first_path,a);
+        assert(first && first!=&a && state.depth==1);
+        { ModDiscReadSession inner;
+          assert(mod_disc_read_reader(first_path,b)==first && state.depth==2); }
+        assert(state.depth==1 && mod_disc_read_reader(first_path,c)==first);
+        assert(!mod_disc_read_reader("missing-disc-must-not-exist.cue",c));
+        assert(!state.reader && state.mount.empty());
+        assert(mod_disc_read_reader(second_path,b) && state.mount==second_path);
+        assert(mod_disc_read_reader(first_path,c) && state.mount==first_path);
+    }
+    assert(!state.depth && !state.reader && state.mount.empty());
+    try { ModDiscReadSession scope; assert(mod_disc_read_reader(first_path,fresh)); throw std::runtime_error("test"); }
+    catch(const std::runtime_error&) {}
+    assert(!state.depth && !state.reader);
+    puts("PASS fresh reader outside scope, reuse, nested scopes, missing/change mount, RAII cleanup");
+}