From 75dd4d0aee7092d114b16cb4313fe42538e02659 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 24 Sep 2016 17:27:45 +0200 Subject: [PATCH 1/2] DVDInterface: Make changing discs savestate-safe --- Source/Core/Core/HW/DVDInterface.cpp | 26 ++++++++++++++++++-------- Source/Core/Core/State.cpp | 2 +- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/HW/DVDInterface.cpp b/Source/Core/Core/HW/DVDInterface.cpp index 3599634e8e..2edac531e2 100644 --- a/Source/Core/Core/HW/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVDInterface.cpp @@ -244,6 +244,8 @@ static CoreTiming::EventType* s_dtk; static u64 s_last_read_offset; static u64 s_last_read_time; +static std::string s_disc_path_to_insert; + static CoreTiming::EventType* s_eject_disc; static CoreTiming::EventType* s_insert_disc; @@ -289,6 +291,8 @@ void DoState(PointerWrap& p) p.Do(s_last_read_offset); p.Do(s_last_read_time); + p.Do(s_disc_path_to_insert); + p.Do(s_stop_at_track_end); DVDThread::DoState(p); @@ -389,6 +393,8 @@ void Init() s_last_read_offset = 0; s_last_read_time = 0; + s_disc_path_to_insert.clear(); + s_eject_disc = CoreTiming::RegisterEvent("EjectDisc", EjectDiscCallback); s_insert_disc = CoreTiming::RegisterEvent("InsertDisc", InsertDiscCallback); @@ -458,16 +464,16 @@ static void EjectDiscCallback(u64 userdata, s64 cyclesLate) static void InsertDiscCallback(u64 userdata, s64 cyclesLate) { const std::string& old_path = SConfig::GetInstance().m_strFilename; - std::string* new_path = reinterpret_cast(userdata); - if (!SetVolumeName(*new_path)) + if (!SetVolumeName(s_disc_path_to_insert)) { // Put back the old one SetVolumeName(old_path); - PanicAlertT("Invalid file"); + PanicAlertT("The disc that was about to be inserted couldn't be found."); } SetDiscInside(VolumeIsValid()); - delete new_path; + + s_disc_path_to_insert.clear(); } // Can only be called by the host thread @@ -484,11 +490,15 @@ void ChangeDiscAsHost(const std::string& new_path) // Can only be called by the CPU thread void ChangeDiscAsCPU(const std::string& new_path) { - // TODO: This is bad. Pointers in CoreTiming userdata require - // manual memory management and aren't savestate-safe. - u64 new_path_pointer = reinterpret_cast(new std::string(new_path)); + if (!s_disc_path_to_insert.empty()) + { + PanicAlertT("A disc is already about to be inserted."); + return; + } + + s_disc_path_to_insert = new_path; CoreTiming::ScheduleEvent(0, s_eject_disc); - CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond(), s_insert_disc, new_path_pointer); + CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond(), s_insert_disc); Movie::SignalDiscChange(new_path); } diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index 37e42ba94f..c8801a9373 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -70,7 +70,7 @@ static Common::Event g_compressAndDumpStateSyncEvent; static std::thread g_save_thread; // Don't forget to increase this after doing changes on the savestate system -static const u32 STATE_VERSION = 59; // Last changed in PR 3490 +static const u32 STATE_VERSION = 60; // Last changed in PR 4242 // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list, From d44b2de01da3eb2ebf8a9d1b5aaab156a949475d Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 25 Sep 2016 10:22:35 +0200 Subject: [PATCH 2/2] DVDInterface: Try to enforce disc inside status on savestate load --- Source/Core/Core/HW/DVDInterface.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Source/Core/Core/HW/DVDInterface.cpp b/Source/Core/Core/HW/DVDInterface.cpp index 2edac531e2..728fd8a8c7 100644 --- a/Source/Core/Core/HW/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVDInterface.cpp @@ -296,6 +296,19 @@ void DoState(PointerWrap& p) p.Do(s_stop_at_track_end); DVDThread::DoState(p); + + // s_inserted_volume isn't savestated (because it points to + // files on the local system). Instead, we check that + // s_disc_inside matches the status of s_inserted_volume. + // This won't catch cases of having the wrong disc inserted, though. + // TODO: Check the game ID, disc number, revision? + if (s_disc_inside != (s_inserted_volume != nullptr)) + { + if (s_disc_inside) + PanicAlertT("An inserted disc was expected but not found."); + else + s_inserted_volume.reset(); + } } static u32 ProcessDTKSamples(short* tempPCM, u32 num_samples)