Rewrite raw memcard threading code. Intended to fix issue 7484.

EXI memcard code now doesn't know specifics of how data is flushed to whatever backing storage is used.
GC raw memcard now flushes every 15 seconds if dirty, and on memcard destruction.
GCI folder now flushes only on memcard destruction.

Source/Core/Core/HW/EXI_Channel.cpp

@@ -159,7 +159,7 @@ void CEXIChannel::SendTransferComplete()
 void CEXIChannel::RemoveDevices()
 {
 	for (auto& device : m_pDevices)
-		device.reset();
+		device.reset(nullptr);
 }
 
 void CEXIChannel::AddDevice(const TEXIDevices device_type, const int device_num)

Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp

@@ -33,45 +33,62 @@
 static const u32 MC_TRANSFER_RATE_READ = 512 * 1024;
 static const u32 MC_TRANSFER_RATE_WRITE = (u32)(96.125f * 1024.0f);
 
-void CEXIMemoryCard::FlushCallback(u64 userdata, int cyclesLate)
+// Takes care of the nasty recovery of the 'this' pointer from card_index,
+// stored in the userdata parameter of the CoreTiming event.
+void CEXIMemoryCard::EventCompleteFindInstance(u64 userdata, std::function<void(CEXIMemoryCard*)> callback)
 {
-	// note that userdata is forbidden to be a pointer, due to the implementation of EventDoState
 	int card_index = (int)userdata;
-	CEXIMemoryCard* pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(EXIDEVICE_MEMORYCARD, card_index);
+	CEXIMemoryCard* pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(
+		EXIDEVICE_MEMORYCARD, card_index);
 	if (pThis == nullptr)
-		pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(EXIDEVICE_MEMORYCARDFOLDER, card_index);
-	if (pThis && pThis->memorycard)
-		pThis->memorycard->Flush();
+	{
+		pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(
+			EXIDEVICE_MEMORYCARDFOLDER, card_index);
+	}
+	if (pThis)
+	{
+		callback(pThis);
+	}
 }
 
 void CEXIMemoryCard::CmdDoneCallback(u64 userdata, int cyclesLate)
 {
-	int card_index = (int)userdata;
-	CEXIMemoryCard* pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(EXIDEVICE_MEMORYCARD, card_index);
-	if (pThis == nullptr)
-		pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(EXIDEVICE_MEMORYCARDFOLDER, card_index);
-	if (pThis)
-		pThis->CmdDone();
+	EventCompleteFindInstance(userdata, [](CEXIMemoryCard* instance)
+	{
+		instance->CmdDone();
+	});
 }
 
 void CEXIMemoryCard::TransferCompleteCallback(u64 userdata, int cyclesLate)
 {
-	int card_index = (int)userdata;
-	CEXIMemoryCard* pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(EXIDEVICE_MEMORYCARD, card_index);
-	if (pThis == nullptr)
-		pThis = (CEXIMemoryCard*)ExpansionInterface::FindDevice(EXIDEVICE_MEMORYCARDFOLDER, card_index);
-	if (pThis)
-		pThis->TransferComplete();
+	EventCompleteFindInstance(userdata, [](CEXIMemoryCard* instance)
+	{
+		instance->TransferComplete();
+	});
 }
 
 CEXIMemoryCard::CEXIMemoryCard(const int index, bool gciFolder)
 	: card_index(index)
-	, m_bDirty(false)
 {
-	// we're potentially leaking events here, since there's no UnregisterEvent until emu shutdown, but I guess it's inconsequential
-	et_this_card = CoreTiming::RegisterEvent((index == 0) ? "memcardFlushA" : "memcardFlushB", FlushCallback);
-	et_cmd_done = CoreTiming::RegisterEvent((index == 0) ? "memcardDoneA" : "memcardDoneB", CmdDoneCallback);
-	et_transfer_complete = CoreTiming::RegisterEvent((index == 0) ? "memcardTransferCompleteA" : "memcardTransferCompleteB", TransferCompleteCallback);
+	struct
+	{
+		const char *done;
+		const char *transfer_complete;
+	} const event_names[] = {
+		{ "memcardDoneA", "memcardTransferCompleteA" },
+		{ "memcardDoneB", "memcardTransferCompleteB" },
+	};
+
+	if ((size_t)index >= ArraySize(event_names))
+	{
+		PanicAlertT("Trying to create invalid memory card index.");
+	}
+	// we're potentially leaking events here, since there's no RemoveEvent
+	// until emu shutdown, but I guess it's inconsequential
+	et_cmd_done = CoreTiming::RegisterEvent(event_names[index].done,
+		CmdDoneCallback);
+	et_transfer_complete = CoreTiming::RegisterEvent(
+		event_names[index].transfer_complete, TransferCompleteCallback);
 
 	interruptSwitch = 0;
 	m_bInterruptSet = 0;
@@ -226,9 +243,8 @@ void CEXIMemoryCard::SetupRawMemcard(u16 sizeMb)
 
 CEXIMemoryCard::~CEXIMemoryCard()
 {
-	CoreTiming::RemoveEvent(et_this_card);
-	memorycard->Flush(true);
-	memorycard.reset();
+	CoreTiming::RemoveEvent(et_cmd_done);
+	CoreTiming::RemoveEvent(et_transfer_complete);
 }
 
 bool CEXIMemoryCard::UseDelayedTransferCompletion()
@@ -247,7 +263,6 @@ void CEXIMemoryCard::CmdDone()
 	status &= ~MC_STATUS_BUSY;
 
 	m_bInterruptSet = 1;
-	m_bDirty = true;
 }
 
 void CEXIMemoryCard::TransferComplete()
@@ -264,9 +279,6 @@ void CEXIMemoryCard::CmdDoneLater(u64 cycles)
 
 void CEXIMemoryCard::SetCS(int cs)
 {
-	// So that memory card won't be invalidated during flushing
-	memorycard->JoinThread();
-
 	if (cs)  // not-selected to selected
 	{
 		m_uPosition = 0;
@@ -291,10 +303,10 @@ void CEXIMemoryCard::SetCS(int cs)
 		case cmdChipErase:
 			if (m_uPosition > 2)
 			{
-				// TODO: Investigate on HW, I (LPFaint99) believe that this only erases the system area (Blocks 0-4)
+				// TODO: Investigate on HW, I (LPFaint99) believe that this only
+				// erases the system area (Blocks 0-4)
 				memorycard->ClearAll();
 				status &= ~MC_STATUS_BUSY;
-				m_bDirty = true;
 			}
 			break;
 
@@ -483,16 +495,6 @@ void CEXIMemoryCard::TransferByte(u8 &byte)
 	DEBUG_LOG(EXPANSIONINTERFACE, "EXI MEMCARD: < %02x", byte);
 }
 
-void CEXIMemoryCard::PauseAndLock(bool doLock, bool unpauseOnUnlock)
-{
-	if (doLock)
-	{
-		// we don't exactly have anything to pause,
-		// but let's make sure the flush thread isn't running.
-		memorycard->JoinThread();
-	}
-}
-
 void CEXIMemoryCard::DoState(PointerWrap &p)
 {
 	// for movie sync, we need to save/load memory card contents (and other data) in savestates.
@@ -509,7 +511,6 @@ void CEXIMemoryCard::DoState(PointerWrap &p)
 		p.Do(status);
 		p.Do(m_uPosition);
 		p.Do(programming_buffer);
-		p.Do(m_bDirty);
 		p.Do(address);
 		memorycard->DoState(p);
 		p.Do(card_index);
@@ -531,13 +532,16 @@ void CEXIMemoryCard::DMARead(u32 _uAddr, u32 _uSize)
 {
 	memorycard->Read(address, _uSize, Memory::GetPointer(_uAddr));
 
-#ifdef _DEBUG
 	if ((address + _uSize) % BLOCK_SIZE == 0)
-		INFO_LOG(EXPANSIONINTERFACE, "reading from block: %x", address / BLOCK_SIZE);
-#endif
+	{
+		DEBUG_LOG(EXPANSIONINTERFACE, "reading from block: %x",
+			address / BLOCK_SIZE);
+	}
 
 	// Schedule transfer complete later based on read speed
-	CoreTiming::ScheduleEvent(_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_READ), et_transfer_complete, (u64)card_index);
+	CoreTiming::ScheduleEvent(
+		_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_READ),
+		et_transfer_complete, (u64)card_index);
 }
 
 // DMA write are preceded by all of the necessary setup via IMMWrite
@@ -546,21 +550,14 @@ void CEXIMemoryCard::DMAWrite(u32 _uAddr, u32 _uSize)
 {
 	memorycard->Write(address, _uSize, Memory::GetPointer(_uAddr));
 
-	// At the end of writing to a block flush to disk
-	// memory card blocks are always(?) written as a whole,
-	// but the dma calls are by page size (0x200) at a time
-	// just in case this is the last block that the game will be writing for a while
 	if (((address + _uSize) % BLOCK_SIZE) == 0)
 	{
-		INFO_LOG(EXPANSIONINTERFACE, "writing to block: %x", address / BLOCK_SIZE);
-		// Page written to memory card, not just to buffer - let's schedule a flush 0.5b cycles into the future (1 sec)
-		// But first we unschedule already scheduled flushes - no point in flushing once per page for a large write
-		// Scheduling event is mainly for raw memory cards as the flush the whole 16MB to disk
-		// Flushing the gci folder is free in comparison
-		CoreTiming::RemoveEvent(et_this_card);
-		CoreTiming::ScheduleEvent(500000000, et_this_card, (u64)card_index);
+		DEBUG_LOG(EXPANSIONINTERFACE, "writing to block: %x",
+			address / BLOCK_SIZE);
 	}
 
 	// Schedule transfer complete later based on write speed
-	CoreTiming::ScheduleEvent(_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_WRITE), et_transfer_complete, (u64)card_index);
+	CoreTiming::ScheduleEvent(
+		_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_WRITE),
+		et_transfer_complete, (u64)card_index);
 }

Source/Core/Core/HW/EXI_DeviceMemoryCard.h

@@ -16,7 +16,6 @@ public:
 	bool UseDelayedTransferCompletion() override;
 	bool IsPresent() override;
 	void DoState(PointerWrap &p) override;
-	void PauseAndLock(bool doLock, bool unpauseOnUnlock=true) override;
 	IEXIDevice* FindDevice(TEXIDevices device_type, int customIndex=-1) override;
 	void DMARead(u32 _uAddr, u32 _uSize) override;
 	void DMAWrite(u32 _uAddr, u32 _uSize) override;
@@ -24,9 +23,7 @@ public:
 private:
 	void SetupGciFolder(u16 sizeMb);
 	void SetupRawMemcard(u16 sizeMb);
-	// This is scheduled whenever a page write is issued. The this pointer is passed
-	// through the userdata parameter, so that it can then call Flush on the right card.
-	static void FlushCallback(u64 userdata, int cyclesLate);
+	static void EventCompleteFindInstance(u64 userdata, std::function<void(CEXIMemoryCard*)> callback);
 
 	// Scheduled when a command that required delayed end signaling is done.
 	static void CmdDoneCallback(u64 userdata, int cyclesLate);
@@ -34,9 +31,6 @@ private:
 	// Scheduled when memory card is done transferring data
 	static void TransferCompleteCallback(u64 userdata, int cyclesLate);
 
-	// Flushes the memory card contents to disk.
-	void Flush(bool exiting = false);
-
 	// Signals that the command that was previously executed is now done.
 	void CmdDone();
 
@@ -66,7 +60,7 @@ private:
 	};
 
 	int card_index;
-	int et_this_card, et_cmd_done, et_transfer_complete;
+	int et_cmd_done, et_transfer_complete;
 	//! memory card state
 
 	// STATE_TO_SAVE
@@ -76,7 +70,6 @@ private:
 	int status;
 	u32 m_uPosition;
 	u8 programming_buffer[128];
-	bool m_bDirty;
 	//! memory card parameters
 	unsigned int card_id;
 	unsigned int address;

Source/Core/Core/HW/GCMemcard.h

@@ -70,14 +70,16 @@ public:
 	{
 	}
 	virtual ~MemoryCardBase() {}
-	virtual void Flush(bool exiting = false) = 0;
 	virtual s32 Read(u32 address, s32 length, u8 *destaddress) = 0;
 	virtual s32 Write(u32 destaddress, s32 length, u8 *srcaddress) = 0;
 	virtual void ClearBlock(u32 address) = 0;
 	virtual void ClearAll() = 0;
 	virtual void DoState(PointerWrap &p) = 0;
-	virtual void JoinThread() {};
 	u32 GetCardId() { return nintendo_card_id; }
+	bool IsAddressInBounds(u32 address) const
+	{
+		return address <= (memory_card_size - 1);
+	}
 
 protected:
 	int card_index;

Source/Core/Core/HW/GCMemcardDirectory.cpp

@@ -157,6 +157,11 @@ GCMemcardDirectory::GCMemcardDirectory(std::string directory, int slot, u16 size
 	m_bat2 = m_bat1;
 }
 
+GCMemcardDirectory::~GCMemcardDirectory()
+{
+	FlushToFile();
+}
+
 s32 GCMemcardDirectory::Read(u32 address, s32 length, u8 *destaddress)
 {
 
@@ -468,7 +473,7 @@ bool GCMemcardDirectory::SetUsedBlocks(int saveIndex)
 	return true;
 }
 
-void GCMemcardDirectory::Flush(bool exiting)
+void GCMemcardDirectory::FlushToFile()
 {
 	int errors = 0;
 	DEntry invalid;
@@ -500,21 +505,18 @@ void GCMemcardDirectory::Flush(bool exiting)
 					GCI.WriteBytes(&m_saves[i].m_gci_header, DENTRY_SIZE);
 					GCI.WriteBytes(m_saves[i].m_save_data.data(), BLOCK_SIZE * m_saves[i].m_save_data.size());
 
-					if (!exiting)
+					if (GCI.IsGood())
 					{
-						if (GCI.IsGood())
-						{
-							Core::DisplayMessage(
-								StringFromFormat("Wrote save contents to %s", m_saves[i].m_filename.c_str()), 4000);
-						}
-						else
-						{
-							++errors;
-							Core::DisplayMessage(
-								StringFromFormat("Failed to write save contents to %s", m_saves[i].m_filename.c_str()),
-								4000);
-							ERROR_LOG(EXPANSIONINTERFACE, "Failed to save data to %s", m_saves[i].m_filename.c_str());
-						}
+						Core::DisplayMessage(
+							StringFromFormat("Wrote save contents to %s", m_saves[i].m_filename.c_str()), 4000);
+					}
+					else
+					{
+						++errors;
+						Core::DisplayMessage(
+							StringFromFormat("Failed to write save contents to %s", m_saves[i].m_filename.c_str()),
+							4000);
+						ERROR_LOG(EXPANSIONINTERFACE, "Failed to save data to %s", m_saves[i].m_filename.c_str());
 					}
 				}
 			}

Source/Core/Core/HW/GCMemcardDirectory.h

@@ -15,13 +15,13 @@ class GCMemcardDirectory : public MemoryCardBase, NonCopyable
 public:
 	GCMemcardDirectory(std::string directory, int slot = 0, u16 sizeMb = MemCard2043Mb, bool ascii = true,
 		DiscIO::IVolume::ECountry  card_region = DiscIO::IVolume::COUNTRY_EUROPE, int gameId = 0);
-	~GCMemcardDirectory() { Flush(true); }
-	void Flush(bool exiting = false) override;
+	~GCMemcardDirectory();
+	void FlushToFile();
 
 	s32 Read(u32 address, s32 length, u8 *destaddress) override;
 	s32 Write(u32 destaddress, s32 length, u8 *srcaddress) override;
 	void ClearBlock(u32 address) override;
-	void ClearAll() override { ; }
+	void ClearAll() override {}
 	void DoState(PointerWrap &p) override;
 
 private:

Source/Core/Core/HW/GCMemcardRaw.cpp

@@ -2,7 +2,9 @@
 // Licensed under GPLv2
 // Refer to the license.txt file included.
 
+#include <chrono>
 #include "Common/ChunkFile.h"
+#include "Common/StdMakeUnique.h"
 #include "Core/Core.h"
 #include "Core/HW/GCMemcard.h"
 #include "Core/HW/GCMemcardRaw.h"
@@ -10,53 +12,21 @@
 #define SIZE_TO_Mb (1024 * 8 * 16)
 #define MC_HDR_SIZE 0xA000
 
-static void innerFlush(FlushData *data)
-{
-	File::IOFile pFile(data->filename, "r+b");
-	if (!pFile)
-	{
-		std::string dir;
-		SplitPath(data->filename, &dir, nullptr, nullptr);
-		if (!File::IsDirectory(dir))
-			File::CreateFullPath(dir);
-		pFile.Open(data->filename, "wb");
-	}
-
-	if (!pFile) // Note - pFile changed inside above if
-	{
-		PanicAlertT("Could not write memory card file %s.\n\n"
-					"Are you running Dolphin from a CD/DVD, or is the save file maybe write protected?\n\n"
-					"Are you receiving this after moving the emulator directory?\nIf so, then you may "
-					"need to re-specify your memory card location in the options.",
-					data->filename.c_str());
-		return;
-	}
-
-	pFile.WriteBytes(data->memcardContent, data->memcardSize);
-
-	if (!data->bExiting)
-		Core::DisplayMessage(StringFromFormat("Wrote memory card %c contents to %s", data->memcardIndex ? 'B' : 'A',
-											  data->filename.c_str()).c_str(),
-							 4000);
-	return;
-}
-
 MemoryCard::MemoryCard(std::string filename, int _card_index, u16 sizeMb)
 	: MemoryCardBase(_card_index, sizeMb)
-	, m_bDirty(false)
-	, m_strFilename(filename)
+	, m_filename(filename)
 {
-	File::IOFile pFile(m_strFilename, "rb");
+	File::IOFile pFile(m_filename, "rb");
 	if (pFile)
 	{
-		// Measure size of the memcard file.
-		memory_card_size = (int)pFile.GetSize();
+		// Measure size of the existing memcard file.
+		memory_card_size = (u32)pFile.GetSize();
 		nintendo_card_id = memory_card_size / SIZE_TO_Mb;
-		memory_card_content = new u8[memory_card_size];
-		memset(memory_card_content, 0xFF, memory_card_size);
+		m_memcard_data = std::make_unique<u8[]>(memory_card_size);
+		memset(&m_memcard_data[0], 0xFF, memory_card_size);
 
-		INFO_LOG(EXPANSIONINTERFACE, "Reading memory card %s", m_strFilename.c_str());
-		pFile.ReadBytes(memory_card_content, memory_card_size);
+		INFO_LOG(EXPANSIONINTERFACE, "Reading memory card %s", m_filename.c_str());
+		pFile.ReadBytes(&m_memcard_data[0], memory_card_size);
 	}
 	else
 	{
@@ -64,108 +34,186 @@ MemoryCard::MemoryCard(std::string filename, int _card_index, u16 sizeMb)
 		nintendo_card_id = sizeMb;
 		memory_card_size = sizeMb * SIZE_TO_Mb;
 
-		memory_card_content = new u8[memory_card_size];
-		GCMemcard::Format(memory_card_content, m_strFilename.find(".JAP.raw") != std::string::npos, sizeMb);
-		memset(memory_card_content + MC_HDR_SIZE, 0xFF, memory_card_size - MC_HDR_SIZE);
+		m_memcard_data = std::make_unique<u8[]>(memory_card_size);
+		// Fills in MC_HDR_SIZE bytes
+		GCMemcard::Format(&m_memcard_data[0], m_filename.find(".JAP.raw") != std::string::npos, sizeMb);
+		memset(&m_memcard_data[MC_HDR_SIZE], 0xFF, memory_card_size - MC_HDR_SIZE);
 
-		WARN_LOG(EXPANSIONINTERFACE, "No memory card found. Will create a new one.");
+		INFO_LOG(EXPANSIONINTERFACE, "No memory card found - a new one was created.");
 	}
+
+	// Class members (including inherited ones) have now been initialized, so
+	// it's safe to startup the flush thread (which reads them).
+	m_flush_buffer = std::make_unique<u8[]>(memory_card_size);
+	m_flush_thread = std::thread(&MemoryCard::FlushThread, this);
 }
 
 MemoryCard::~MemoryCard()
 {
-	Flush(true);
-	delete[] memory_card_content;
-}
-
-void MemoryCard::JoinThread()
-{
-	if (flushThread.joinable())
+	if (m_flush_thread.joinable())
 	{
-		flushThread.join();
+		// Update the flush buffer one last time, flush, and join.
+		{
+			std::unique_lock<std::mutex> l(m_flush_mutex);
+			memcpy(&m_flush_buffer[0], &m_memcard_data[0], memory_card_size);
+		}
+
+		m_is_exiting.Set();
+		m_flush_trigger.Set();
+
+		m_flush_thread.join();
 	}
 }
 
-// Flush memory card contents to disc
-void MemoryCard::Flush(bool exiting)
+void MemoryCard::FlushThread()
 {
-	if (!m_bDirty)
-		return;
-
 	if (!Core::g_CoreStartupParameter.bEnableMemcardSaving)
-		return;
-
-	if (flushThread.joinable())
 	{
-		flushThread.join();
+		return;
 	}
 
-	if (!exiting)
-		Core::DisplayMessage(StringFromFormat("Writing to memory card %c", card_index ? 'B' : 'A'), 1000);
+	Common::SetCurrentThreadName(
+		StringFromFormat("Memcard%x-Flush", card_index).c_str());
 
-	flushData.filename = m_strFilename;
-	flushData.memcardContent = memory_card_content;
-	flushData.memcardIndex = card_index;
-	flushData.memcardSize = memory_card_size;
-	flushData.bExiting = exiting;
+	const auto flush_interval = std::chrono::seconds(15);
+	auto last_flush = std::chrono::steady_clock::now();
+	bool dirty = false;
 
-	flushThread = std::thread(innerFlush, &flushData);
-	if (exiting)
-		flushThread.join();
+	for (;;)
+	{
+		bool triggered = m_flush_trigger.WaitFor(flush_interval);
+		bool do_exit = m_is_exiting.IsSet();
+		if (triggered)
+		{
+			dirty = true;
+		}
+		// Delay the flush if we're not exiting or if the event timed out and
+		// the state isn't dirty.
+		if (!do_exit)
+		{
+			auto now = std::chrono::steady_clock::now();
+			if (now - last_flush < flush_interval || !dirty)
+			{
+				continue;
+			}
+			last_flush = now;
+		}
 
-	m_bDirty = false;
+		// Opening the file is purposefully done each iteration to ensure the
+		// file doesn't disappear out from under us after the first check.
+		File::IOFile pFile(m_filename, "r+b");
+
+		if (!pFile)
+		{
+			std::string dir;
+			SplitPath(m_filename, &dir, nullptr, nullptr);
+			if (!File::IsDirectory(dir))
+			{
+				File::CreateFullPath(dir);
+			}
+			pFile.Open(m_filename, "wb");
+		}
+
+		// Note - pFile may have changed above, after ctor
+		if (!pFile)
+		{
+			PanicAlertT(
+				"Could not write memory card file %s.\n\n"
+				"Are you running Dolphin from a CD/DVD, or is the save file maybe write protected?\n\n"
+				"Are you receiving this after moving the emulator directory?\nIf so, then you may "
+				"need to re-specify your memory card location in the options.",
+				m_filename.c_str());
+
+			// Exit the flushing thread - further flushes will be ignored unless
+			// the thread is recreated.
+			return;
+		}
+
+		{
+			std::unique_lock<std::mutex> l(m_flush_mutex);
+			pFile.WriteBytes(&m_flush_buffer[0], memory_card_size);
+		}
+
+		dirty = false;
+
+		if (!do_exit)
+		{
+			Core::DisplayMessage(
+				StringFromFormat("Wrote memory card %c contents to %s",
+				card_index ? 'B' : 'A', m_filename.c_str()).c_str(),
+				4000);
+		}
+		else
+		{
+			return;
+		}
+	}
+}
+
+// Attempt to update the flush buffer and trigger a flush. If we can't get a
+// lock in order to update the flush buffer, a write is in progress and a future
+// write will take care of any changes to the memcard data, so nothing needs to
+// be done now.
+void MemoryCard::TryFlush()
+{
+	if (m_flush_mutex.try_lock())
+	{
+		memcpy(&m_flush_buffer[0], &m_memcard_data[0], memory_card_size);
+		m_flush_mutex.unlock();
+		m_flush_trigger.Set();
+	}
 }
 
 s32 MemoryCard::Read(u32 srcaddress, s32 length, u8 *destaddress)
 {
-	if (!memory_card_content)
-		return -1;
-	if (srcaddress > (memory_card_size - 1))
+	if (!IsAddressInBounds(srcaddress))
 	{
-		PanicAlertT("MemoryCard: Read called with invalid source address, %x", srcaddress);
+		PanicAlertT("MemoryCard: Read called with invalid source address, %x",
+					srcaddress);
 		return -1;
 	}
 
-	memcpy(destaddress, &(memory_card_content[srcaddress]), length);
+	memcpy(destaddress, &m_memcard_data[srcaddress], length);
 	return length;
 }
 
 s32 MemoryCard::Write(u32 destaddress, s32 length, u8 *srcaddress)
 {
-	if (!memory_card_content)
-		return -1;
-
-	if (destaddress > (memory_card_size - 1))
+	if (!IsAddressInBounds(destaddress))
 	{
-		PanicAlertT("MemoryCard: Write called with invalid destination address, %x", destaddress);
+		PanicAlertT("MemoryCard: Write called with invalid destination address, %x",
+					destaddress);
 		return -1;
 	}
 
-	m_bDirty = true;
-	memcpy(&(memory_card_content[destaddress]), srcaddress, length);
+	memcpy(&m_memcard_data[destaddress], srcaddress, length);
+	TryFlush();
 	return length;
 }
 
 void MemoryCard::ClearBlock(u32 address)
 {
-	if (address & (BLOCK_SIZE - 1) || address > (memory_card_size - 1))
-		PanicAlertT("MemoryCard: ClearBlock called on invalid address %x", address);
+	if (address & (BLOCK_SIZE - 1) || !IsAddressInBounds(address))
+	{
+		PanicAlertT("MemoryCard: ClearBlock called on invalid address %x",
+					address);
+	}
 	else
 	{
-		m_bDirty = true;
-		memset(memory_card_content + address, 0xFF, BLOCK_SIZE);
+		memset(&m_memcard_data[address], 0xFF, BLOCK_SIZE);
+		TryFlush();
 	}
 }
 
 void MemoryCard::ClearAll()
 {
-	m_bDirty = true;
-	memset(memory_card_content, 0xFF, memory_card_size);
+	memset(&m_memcard_data[0], 0xFF, memory_card_size);
+	TryFlush();
 }
 
 void MemoryCard::DoState(PointerWrap &p)
 {
 	p.Do(card_index);
 	p.Do(memory_card_size);
-	p.DoArray(memory_card_content, memory_card_size);
+	p.DoArray(&m_memcard_data[0], memory_card_size);
 }

Source/Core/Core/HW/GCMemcardRaw.h

@@ -4,39 +4,34 @@
 
 #pragma once
 
+#include <memory>
+#include "Common/Event.h"
+#include "Common/Flag.h"
 #include "Common/Thread.h"
 #include "Core/HW/GCMemcard.h"
 
 class PointerWrap;
 
-// Data structure to be passed to the flushing thread.
-struct FlushData
-{
-	bool bExiting;
-	std::string filename;
-	u8 *memcardContent;
-	int memcardSize, memcardIndex;
-};
-
 class MemoryCard : public MemoryCardBase
 {
 public:
 	MemoryCard(std::string filename, int _card_index, u16 sizeMb = MemCard2043Mb);
 	~MemoryCard();
-	void Flush(bool exiting = false) override;
+	void FlushThread();
+	void TryFlush();
 
 	s32 Read(u32 address, s32 length, u8 *destaddress) override;
 	s32 Write(u32 destaddress, s32 length, u8 *srcaddress) override;
 	void ClearBlock(u32 address) override;
 	void ClearAll() override;
 	void DoState(PointerWrap &p) override;
-	void JoinThread() override;
 
 private:
-	u8 *memory_card_content;
-	bool m_bDirty;
-	std::string m_strFilename;
-
-	FlushData flushData;
-	std::thread flushThread;
+	std::string m_filename;
+	std::unique_ptr<u8[]> m_memcard_data;
+	std::thread m_flush_thread;
+	std::mutex m_flush_mutex;
+	Common::Event m_flush_trigger;
+	Common::Flag m_is_exiting;
+	std::unique_ptr<u8[]> m_flush_buffer;
 };

Source/Core/Core/State.cpp

@@ -63,7 +63,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 = 30;
+static const u32 STATE_VERSION = 31;
 
 enum
 {