Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #10235 from AdmiralCurtiss/netplay-save-sync-boot 
Netplay: Fix possible Wii save restore race condition between Netplay and CPU threads on game shutdown by making the Wii Save Sync data part of the BootParameters.
  • Loading branch information
@leoetlino
leoetlino committed Nov 23, 2021
2 parents cc9a75a + 6350c93 commit ba62019
Show file tree
Hide file tree
Showing 17 changed files with 259 additions and 113 deletions.
14 changes: 7 additions & 7 deletions Source/Android/jni/MainAndroid.cpp
View file
Expand Up @@ -549,8 +549,7 @@ static float GetRenderSurfaceScale(JNIEnv* env)
}

static void Run(JNIEnv* env, const std::vector<std::string>& paths, bool riivolution,
const std::optional<std::string>& savestate_path = {},
bool delete_savestate = false)
BootSessionData boot_session_data = BootSessionData())
{
ASSERT(!paths.empty());
__android_log_print(ANDROID_LOG_INFO, DOLPHIN_TAG, "Running : %s", paths[0].c_str());
Expand All @@ -561,9 +560,8 @@ static void Run(JNIEnv* env, const std::vector<std::string>& paths, bool riivolu

s_have_wm_user_stop = false;

std::unique_ptr<BootParameters> boot = BootParameters::GenerateFromFile(paths, savestate_path);
if (boot)
boot->delete_savestate = delete_savestate;
std::unique_ptr<BootParameters> boot =
BootParameters::GenerateFromFile(paths, std::move(boot_session_data));

if (riivolution && std::holds_alternative<BootParameters::Disc>(boot->parameters))
{
Expand Down Expand Up @@ -638,8 +636,10 @@ Java_org_dolphinemu_dolphinemu_NativeLibrary_Run___3Ljava_lang_String_2ZLjava_la
JNIEnv* env, jclass, jobjectArray jPaths, jboolean jRiivolution, jstring jSavestate,
jboolean jDeleteSavestate)
{
Run(env, JStringArrayToVector(env, jPaths), jRiivolution, GetJString(env, jSavestate),
jDeleteSavestate);
DeleteSavestateAfterBoot delete_state =
jDeleteSavestate ? DeleteSavestateAfterBoot::Yes : DeleteSavestateAfterBoot::No;
Run(env, JStringArrayToVector(env, jPaths), jRiivolution,
BootSessionData(GetJString(env, jSavestate), delete_state));
}

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ChangeDisc(JNIEnv* env, jclass,
Expand Down
87 changes: 71 additions & 16 deletions Source/Core/Core/Boot/Boot.cpp
View file
Expand Up @@ -112,22 +112,77 @@ static std::vector<std::string> ReadM3UFile(const std::string& m3u_path,
return result;
}

BootParameters::BootParameters(Parameters&& parameters_,
const std::optional<std::string>& savestate_path_)
: parameters(std::move(parameters_)), savestate_path(savestate_path_)
BootSessionData::BootSessionData()
{
}

std::unique_ptr<BootParameters>
BootParameters::GenerateFromFile(std::string boot_path,
const std::optional<std::string>& savestate_path)
BootSessionData::BootSessionData(std::optional<std::string> savestate_path,
DeleteSavestateAfterBoot delete_savestate)
: m_savestate_path(std::move(savestate_path)), m_delete_savestate(delete_savestate)
{
return GenerateFromFile(std::vector<std::string>{std::move(boot_path)}, savestate_path);
}

std::unique_ptr<BootParameters>
BootParameters::GenerateFromFile(std::vector<std::string> paths,
const std::optional<std::string>& savestate_path)
BootSessionData::BootSessionData(BootSessionData&& other) = default;

BootSessionData& BootSessionData::operator=(BootSessionData&& other) = default;

BootSessionData::~BootSessionData() = default;

const std::optional<std::string>& BootSessionData::GetSavestatePath() const
{
return m_savestate_path;
}

DeleteSavestateAfterBoot BootSessionData::GetDeleteSavestate() const
{
return m_delete_savestate;
}

void BootSessionData::SetSavestateData(std::optional<std::string> savestate_path,
DeleteSavestateAfterBoot delete_savestate)
{
m_savestate_path = std::move(savestate_path);
m_delete_savestate = delete_savestate;
}

IOS::HLE::FS::FileSystem* BootSessionData::GetWiiSyncFS() const
{
return m_wii_sync_fs.get();
}

const std::vector<u64>& BootSessionData::GetWiiSyncTitles() const
{
return m_wii_sync_titles;
}

void BootSessionData::InvokeWiiSyncCleanup() const
{
if (m_wii_sync_cleanup)
m_wii_sync_cleanup();
}

void BootSessionData::SetWiiSyncData(std::unique_ptr<IOS::HLE::FS::FileSystem> fs,
std::vector<u64> titles, WiiSyncCleanupFunction cleanup)
{
m_wii_sync_fs = std::move(fs);
m_wii_sync_titles = std::move(titles);
m_wii_sync_cleanup = std::move(cleanup);
}

BootParameters::BootParameters(Parameters&& parameters_, BootSessionData boot_session_data_)
: parameters(std::move(parameters_)), boot_session_data(std::move(boot_session_data_))
{
}

std::unique_ptr<BootParameters> BootParameters::GenerateFromFile(std::string boot_path,
BootSessionData boot_session_data_)
{
return GenerateFromFile(std::vector<std::string>{std::move(boot_path)},
std::move(boot_session_data_));
}

std::unique_ptr<BootParameters> BootParameters::GenerateFromFile(std::vector<std::string> paths,
BootSessionData boot_session_data_)
{
ASSERT(!paths.empty());

Expand Down Expand Up @@ -176,21 +231,21 @@ BootParameters::GenerateFromFile(std::vector<std::string> paths,
if (disc)
{
return std::make_unique<BootParameters>(Disc{std::move(path), std::move(disc), paths},
savestate_path);
std::move(boot_session_data_));
}

if (extension == ".elf")
{
auto elf_reader = std::make_unique<ElfReader>(path);
return std::make_unique<BootParameters>(Executable{std::move(path), std::move(elf_reader)},
savestate_path);
std::move(boot_session_data_));
}

if (extension == ".dol")
{
auto dol_reader = std::make_unique<DolReader>(path);
return std::make_unique<BootParameters>(Executable{std::move(path), std::move(dol_reader)},
savestate_path);
std::move(boot_session_data_));
}

if (is_drive)
Expand All @@ -209,21 +264,21 @@ BootParameters::GenerateFromFile(std::vector<std::string> paths,
}

if (extension == ".dff")
return std::make_unique<BootParameters>(DFF{std::move(path)}, savestate_path);
return std::make_unique<BootParameters>(DFF{std::move(path)}, std::move(boot_session_data_));

if (extension == ".wad")
{
std::unique_ptr<DiscIO::VolumeWAD> wad = DiscIO::CreateWAD(std::move(path));
if (wad)
return std::make_unique<BootParameters>(std::move(*wad), savestate_path);
return std::make_unique<BootParameters>(std::move(*wad), std::move(boot_session_data_));
}

if (extension == ".json")
{
auto descriptor = DiscIO::ParseGameModDescriptorFile(path);
if (descriptor)
{
auto boot_params = GenerateFromFile(descriptor->base_file, savestate_path);
auto boot_params = GenerateFromFile(descriptor->base_file, std::move(boot_session_data_));
if (!boot_params)
{
PanicAlertFmtT("Could not recognize file {0}", descriptor->base_file);
Expand Down
54 changes: 49 additions & 5 deletions Source/Core/Core/Boot/Boot.h
View file
Expand Up @@ -24,6 +24,11 @@ namespace File
class IOFile;
}

namespace IOS::HLE::FS
{
class FileSystem;
}

struct RegionSetting
{
std::string area;
Expand All @@ -34,6 +39,46 @@ struct RegionSetting

class BootExecutableReader;

enum class DeleteSavestateAfterBoot : u8
{
No,
Yes
};

class BootSessionData
{
public:
BootSessionData();
BootSessionData(std::optional<std::string> savestate_path,
DeleteSavestateAfterBoot delete_savestate);
BootSessionData(const BootSessionData& other) = delete;
BootSessionData(BootSessionData&& other);
BootSessionData& operator=(const BootSessionData& other) = delete;
BootSessionData& operator=(BootSessionData&& other);
~BootSessionData();

const std::optional<std::string>& GetSavestatePath() const;
DeleteSavestateAfterBoot GetDeleteSavestate() const;
void SetSavestateData(std::optional<std::string> savestate_path,
DeleteSavestateAfterBoot delete_savestate);

using WiiSyncCleanupFunction = std::function<void()>;

IOS::HLE::FS::FileSystem* GetWiiSyncFS() const;
const std::vector<u64>& GetWiiSyncTitles() const;
void InvokeWiiSyncCleanup() const;
void SetWiiSyncData(std::unique_ptr<IOS::HLE::FS::FileSystem> fs, std::vector<u64> titles,
WiiSyncCleanupFunction cleanup);

private:
std::optional<std::string> m_savestate_path;
DeleteSavestateAfterBoot m_delete_savestate = DeleteSavestateAfterBoot::No;

std::unique_ptr<IOS::HLE::FS::FileSystem> m_wii_sync_fs;
std::vector<u64> m_wii_sync_titles;
WiiSyncCleanupFunction m_wii_sync_cleanup;
};

struct BootParameters
{
struct Disc
Expand Down Expand Up @@ -70,18 +115,17 @@ struct BootParameters
};

static std::unique_ptr<BootParameters>
GenerateFromFile(std::string boot_path, const std::optional<std::string>& savestate_path = {});
GenerateFromFile(std::string boot_path, BootSessionData boot_session_data_ = BootSessionData());
static std::unique_ptr<BootParameters>
GenerateFromFile(std::vector<std::string> paths,
const std::optional<std::string>& savestate_path = {});
BootSessionData boot_session_data_ = BootSessionData());

using Parameters = std::variant<Disc, Executable, DiscIO::VolumeWAD, NANDTitle, IPL, DFF>;
BootParameters(Parameters&& parameters_, const std::optional<std::string>& savestate_path_ = {});
BootParameters(Parameters&& parameters_, BootSessionData boot_session_data_ = BootSessionData());

Parameters parameters;
std::vector<DiscIO::Riivolution::Patch> riivolution_patches;
std::optional<std::string> savestate_path;
bool delete_savestate = false;
BootSessionData boot_session_data;
};

class CBoot
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/Core/BootManager.cpp
View file
Expand Up @@ -450,7 +450,7 @@ bool BootCore(std::unique_ptr<BootParameters> boot, const WindowSystemInfo& wsi)
std::make_unique<BootParameters>(
BootParameters::IPL{StartUp.m_region,
std::move(std::get<BootParameters::Disc>(boot->parameters))},
boot->savestate_path),
std::move(boot->boot_session_data)),
wsi);
}
return Core::Init(std::move(boot), wsi);
Expand Down
13 changes: 9 additions & 4 deletions Source/Core/Core/Core.cpp
View file
Expand Up @@ -479,8 +479,10 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
Keyboard::LoadConfig();
}

const std::optional<std::string> savestate_path = boot->savestate_path;
const bool delete_savestate = boot->delete_savestate;
BootSessionData boot_session_data = std::move(boot->boot_session_data);
const std::optional<std::string>& savestate_path = boot_session_data.GetSavestatePath();
const bool delete_savestate =
boot_session_data.GetDeleteSavestate() == DeleteSavestateAfterBoot::Yes;

// Load and Init Wiimotes - only if we are booting in Wii mode
bool init_wiimotes = false;
Expand Down Expand Up @@ -615,9 +617,12 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
// Initialise Wii filesystem contents.
// This is done here after Boot and not in BootManager to ensure that we operate
// with the correct title context since save copying requires title directories to exist.
Common::ScopeGuard wiifs_guard{&Core::CleanUpWiiFileSystemContents};
Common::ScopeGuard wiifs_guard{[&boot_session_data] {
Core::CleanUpWiiFileSystemContents(boot_session_data);
boot_session_data.InvokeWiiSyncCleanup();
}};
if (SConfig::GetInstance().bWii)
Core::InitializeWiiFileSystemContents(savegame_redirect);
Core::InitializeWiiFileSystemContents(savegame_redirect, boot_session_data);
else
wiifs_guard.Dismiss();

Expand Down
50 changes: 17 additions & 33 deletions Source/Core/Core/NetPlayClient.cpp
View file
Expand Up @@ -33,6 +33,7 @@
#include "Common/Version.h"

#include "Core/ActionReplay.h"
#include "Core/Boot/Boot.h"
#include "Core/Config/MainSettings.h"
#include "Core/Config/NetplaySettings.h"
#include "Core/Config/SessionSettings.h"
Expand Down Expand Up @@ -75,8 +76,6 @@ using namespace WiimoteCommon;

static std::mutex crit_netplay_client;
static NetPlayClient* netplay_client = nullptr;
static std::unique_ptr<IOS::HLE::FS::FileSystem> s_wii_sync_fs;
static std::vector<u64> s_wii_sync_titles;
static bool s_si_poll_batching = false;

// called from ---GUI--- thread
Expand Down Expand Up @@ -1191,7 +1190,7 @@ void NetPlayClient::OnSyncSaveDataWii(sf::Packet& packet)
}
}

SetWiiSyncData(std::move(temp_fs), titles);
SetWiiSyncData(std::move(temp_fs), std::move(titles));
SyncSaveDataResponse(true);
}

Expand Down Expand Up @@ -1721,7 +1720,14 @@ bool NetPlayClient::StartGame(const std::string& path)
}

// boot game
m_dialog->BootGame(path);
auto boot_session_data = std::make_unique<BootSessionData>();
boot_session_data->SetWiiSyncData(std::move(m_wii_sync_fs), std::move(m_wii_sync_titles), [] {
// on emulation end clean up the Wii save sync directory -- see OnSyncSaveDataWii()
const std::string path = File::GetUserPath(D_USER_IDX) + "Wii" GC_MEMCARD_NETPLAY DIR_SEP;
if (File::Exists(path))
File::DeleteDirRecursively(path);
});
m_dialog->BootGame(path, std::move(boot_session_data));

UpdateDevices();

Expand Down Expand Up @@ -2251,8 +2257,6 @@ bool NetPlayClient::StopGame()
// stop game
m_dialog->StopGame();

ClearWiiSyncData();

return true;
}

Expand Down Expand Up @@ -2496,6 +2500,13 @@ void NetPlayClient::AdjustPadBufferSize(const unsigned int size)
m_dialog->OnPadBufferChanged(size);
}

void NetPlayClient::SetWiiSyncData(std::unique_ptr<IOS::HLE::FS::FileSystem> fs,
std::vector<u64> titles)
{
m_wii_sync_fs = std::move(fs);
m_wii_sync_titles = std::move(titles);
}

SyncIdentifier NetPlayClient::GetSDCardIdentifier()
{
return SyncIdentifier{{}, "sd", {}, {}, {}, {}};
Expand Down Expand Up @@ -2538,33 +2549,6 @@ const NetSettings& GetNetSettings()
return netplay_client->GetNetSettings();
}

IOS::HLE::FS::FileSystem* GetWiiSyncFS()
{
return s_wii_sync_fs.get();
}

const std::vector<u64>& GetWiiSyncTitles()
{
return s_wii_sync_titles;
}

void SetWiiSyncData(std::unique_ptr<IOS::HLE::FS::FileSystem> fs, const std::vector<u64>& titles)
{
s_wii_sync_fs = std::move(fs);
s_wii_sync_titles.insert(s_wii_sync_titles.end(), titles.begin(), titles.end());
}

void ClearWiiSyncData()
{
// We're just assuming it will always be here because it is
const std::string path = File::GetUserPath(D_USER_IDX) + "Wii" GC_MEMCARD_NETPLAY DIR_SEP;
if (File::Exists(path))
File::DeleteDirRecursively(path);

s_wii_sync_fs.reset();
s_wii_sync_titles.clear();
}

void SetSIPollBatching(bool state)
{
s_si_poll_batching = state;
Expand Down

0 comments on commit ba62019

Please sign in to comment.