From cd19d5392e4d76c372e6b9cefd24f927b51f5b11 Mon Sep 17 00:00:00 2001
From: ghost <void.ghost@yahoo.com>
Date: Sat, 26 Sep 2015 17:59:27 +0300
Subject: [PATCH] SaveState: Fix for race condition ("wait" didn't actually
 waited for file to flush/close).

g_compressAndDumpStateSyncEvent was Set() before destruction of file object (i.e. before flushing changes and closing file).

Also, adds Common::ScopeGuard wrapper for RAII.
---
 Source/Core/Common/Common.vcxproj         |  1 +
 Source/Core/Common/Common.vcxproj.filters |  1 +
 Source/Core/Common/ScopeGuard.h           | 50 +++++++++++++++++++++++
 Source/Core/Core/State.cpp                | 17 ++++++--
 4 files changed, 66 insertions(+), 3 deletions(-)
 create mode 100644 Source/Core/Common/ScopeGuard.h

diff --git a/Source/Core/Common/Common.vcxproj b/Source/Core/Common/Common.vcxproj
index fe95d75289..c81bb3ab04 100644
--- a/Source/Core/Common/Common.vcxproj
+++ b/Source/Core/Common/Common.vcxproj
@@ -108,6 +108,7 @@
     <ClInclude Include="Network.h" />
     <ClInclude Include="PcapFile.h" />
     <ClInclude Include="Profiler.h" />
+    <ClInclude Include="ScopeGuard.h" />
     <ClInclude Include="SDCardUtil.h" />
     <ClInclude Include="SettingsHandler.h" />
     <ClInclude Include="StringUtil.h" />
diff --git a/Source/Core/Common/Common.vcxproj.filters b/Source/Core/Common/Common.vcxproj.filters
index a55b5d26d6..f9f84f8426 100644
--- a/Source/Core/Common/Common.vcxproj.filters
+++ b/Source/Core/Common/Common.vcxproj.filters
@@ -53,6 +53,7 @@
     <ClInclude Include="Network.h" />
     <ClInclude Include="PcapFile.h" />
     <ClInclude Include="Profiler.h" />
+    <ClInclude Include="ScopeGuard.h" />
     <ClInclude Include="SDCardUtil.h" />
     <ClInclude Include="SettingsHandler.h" />
     <ClInclude Include="StringUtil.h" />
diff --git a/Source/Core/Common/ScopeGuard.h b/Source/Core/Common/ScopeGuard.h
new file mode 100644
index 0000000000..ab021a178c
--- /dev/null
+++ b/Source/Core/Common/ScopeGuard.h
@@ -0,0 +1,50 @@
+// Copyright 2015 Dolphin Emulator Project
+// Licensed under GPLv2+
+// Refer to the license.txt file included.
+
+#pragma once
+
+#include <functional>
+
+namespace Common
+{
+
+class ScopeGuard final
+{
+public:
+	template<class Callable>
+	ScopeGuard(Callable&& finalizer) : m_finalizer(std::forward<Callable>(finalizer)) {}
+
+	ScopeGuard(ScopeGuard&& other) : m_finalizer(std::move(other.m_finalizer))
+	{
+		other.m_finalizer = nullptr;
+	}
+
+	~ScopeGuard()
+	{
+		Exit();
+	}
+
+	void Dismiss()
+	{
+		m_finalizer = nullptr;
+	}
+
+	void Exit()
+	{
+		if (m_finalizer)
+		{
+			m_finalizer(); // must not throw
+			m_finalizer = nullptr;
+		}
+	}
+
+	ScopeGuard(const ScopeGuard&) = delete;
+
+	void operator=(const ScopeGuard&) = delete;
+
+private:
+	std::function<void()> m_finalizer;
+};
+
+}  // Namespace Common
diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp
index e2a110568f..96e9d7ea1b 100644
--- a/Source/Core/Core/State.cpp
+++ b/Source/Core/Core/State.cpp
@@ -9,6 +9,7 @@
 
 #include "Common/CommonTypes.h"
 #include "Common/Event.h"
+#include "Common/ScopeGuard.h"
 #include "Common/StringUtil.h"
 #include "Common/Thread.h"
 #include "Common/Timer.h"
@@ -280,8 +281,20 @@ struct CompressAndDumpState_args
 static void CompressAndDumpState(CompressAndDumpState_args save_args)
 {
 	std::lock_guard<std::mutex> lk(*save_args.buffer_mutex);
-	if (!save_args.wait)
+
+	// ScopeGuard is used here to ensure that g_compressAndDumpStateSyncEvent.Set()
+	// will be called and that it will happen after the IOFile is closed.
+	// Both ScopeGuard's and IOFile's finalization occur at respective object destruction time.
+	// As Local (stack) objects are destructed in the reverse order of construction and "ScopeGuard on_exit"
+	// is created before the "IOFile f", it is guaranteed that the file will be finalized before
+	// the ScopeGuard's finalization (i.e. "g_compressAndDumpStateSyncEvent.Set()" call).
+	Common::ScopeGuard on_exit([]()
+	{
 		g_compressAndDumpStateSyncEvent.Set();
+	});
+	// If it is not required to wait, we call finalizer early (and it won't be called again at destruction).
+	if (!save_args.wait)
+		on_exit.Exit();
 
 	const u8* const buffer_data = &(*(save_args.buffer_vector))[0];
 	const size_t buffer_size = (save_args.buffer_vector)->size();
@@ -313,7 +326,6 @@ static void CompressAndDumpState(CompressAndDumpState_args save_args)
 	if (!f)
 	{
 		Core::DisplayMessage("Could not save state", 2000);
-		g_compressAndDumpStateSyncEvent.Set();
 		return;
 	}
 
@@ -361,7 +373,6 @@ static void CompressAndDumpState(CompressAndDumpState_args save_args)
 	}
 
 	Core::DisplayMessage(StringFromFormat("Saved State to %s", filename.c_str()), 2000);
-	g_compressAndDumpStateSyncEvent.Set();
 }
 
 void SaveAs(const std::string& filename, bool wait)