Progress: Dialog: Fixup memory management

rpcs3/Emu/scoped_progress_dialog.cpp

@@ -10,17 +10,36 @@ shared_ptr<std::string> progress_dialog_string_t::get_string_ptr() const noexcep
 	{
 		text.reset();
 
+		if (old_val.text_count == 0)
+		{
+			return text;
+		}
+
 		if (old_val.text_index != umax)
 		{
-			if (auto ptr = g_progr_text_queue[old_val.text_index].load())
+			if (text = g_progr_text_queue[old_val.text_index].load(); !text)
 			{
-				text = ptr;
+				// Search for available text (until the time another thread sets index)
+				const u64 queue_size = g_progr_text_queue.size();
+
+				for (u64 i = queue_size - 1;; i--)
+				{
+					if (i == umax)
+					{
+						return text;
+					}
+
+					if ((text = g_progr_text_queue[i].load()))
+					{
+						return text;
+					}
+				}
 			}
 		}
 
 		auto new_val = data.load();
 
-		if (old_val.text_index == new_val.text_index)
+		if (old_val.update_id == new_val.update_id)
 		{
 			break;
 		}
@@ -62,86 +81,36 @@ scoped_progress_dialog::scoped_progress_dialog(std::string text) noexcept
 		}
 	}
 
-	usz unmap_index = umax;
-
 	g_progr_text.data.atomic_op([&](std::common_type_t<progress_dialog_string_t::data_t>& progr)
 	{
-		unmap_index = progr.text_index;
 		progr.update_id++;
 		progr.text_index = m_text_index;
+		progr.text_count++;
 	});
-
-	// Note: unmap_index may be m_text_index if picked up at the destructor
-	// Which is technically the newest value to be inserted so it serves its logic
-	if (unmap_index != umax && unmap_index != m_text_index)
-	{
-		g_progr_text_queue[unmap_index].reset();
-	}
 }
 
 scoped_progress_dialog& scoped_progress_dialog::operator=(std::string text) noexcept
 {
-	shared_ptr<std::string> installed_ptr = make_single_value(std::move(text));
+	// Exchange text atomically
-	const shared_ptr<std::string> c_null_ptr;
+	g_progr_text_queue[m_text_index].exchange(make_single_value(std::move(text)));
-
-	for (usz init_size = g_progr_text_queue.size(), new_text_index = std::max<usz>(init_size, 1) - 1;;)
-	{
-		if (new_text_index >= init_size)
-		{
-			// Search element in new memeory
-			new_text_index++;
-		}
-		else if (new_text_index == 0)
-		{
-			// Search element in new memeory
-			new_text_index = init_size;
-		}
-		else
-		{
-			new_text_index--;
-		}
-
-		auto& info = g_progr_text_queue[new_text_index];
-
-		if (!info && info.compare_and_swap_test(c_null_ptr, installed_ptr))
-		{
-			m_text_index = new_text_index;
-			break;
-		}
-	}
-
-	usz unmap_index = umax;
-
-	g_progr_text.data.atomic_op([&](std::common_type_t<progress_dialog_string_t::data_t>& progr)
-	{
-		unmap_index = progr.text_index;
-		progr.update_id++;
-		progr.text_index = m_text_index;
-	});
-
-	// Note: unmap_index may be m_text_index if picked up at the destructor
-	// Which is technically the newest value to be inserted so it serves its logic
-	if (unmap_index != umax && unmap_index != m_text_index)
-	{
-		g_progr_text_queue[unmap_index].reset();
-	}
-
 	return *this;
 }
 
 scoped_progress_dialog::~scoped_progress_dialog() noexcept
 {
-	bool unmap = false;
+	// Deallocate memory
+	g_progr_text_queue[m_text_index].reset();
+
+	auto progr = g_progr_text.data.load();
 
 	while (true)
 	{
-		auto progr = g_progr_text.data.load();
+		u32 restored_text_index = umax;
-		usz restored_text_index = umax;
 
-		if (progr.text_index == m_text_index)
+		if (progr.text_index == m_text_index && !g_progr_text_queue[m_text_index])
 		{
 			// Search for available text
-			// Out of scope of atomic to keep atomic_op as clean as possible (for potential future enhacements)
+			// Out of scope of atomic loop to keep atomic_op as clean as possible (for potential future enhacements)
 			const u64 queue_size = g_progr_text_queue.size();
 
 			for (u64 i = queue_size - 1;; i--)
@@ -152,11 +121,6 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept
 					break;
 				}
 
-				if (i == m_text_index)
-				{
-					continue;
-				}
-
 				if (g_progr_text_queue[i].operator bool())
 				{
 					restored_text_index = i;
@@ -165,7 +129,7 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept
 			}
 		}
 
-		if (!g_progr_text.data.atomic_op([&](std::common_type_t<progress_dialog_string_t::data_t>& progr0)
+		auto [old, ok] = g_progr_text.data.fetch_op([&](std::common_type_t<progress_dialog_string_t::data_t>& progr0)
 		{
 			if (progr.update_id != progr0.update_id)
 			{
@@ -173,16 +137,14 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept
 				return false;
 			}
 
-			unmap = false;
+			progr0.text_count--;
 
 			if (progr0.text_index == m_text_index)
 			{
-				unmap = true;
-
 				if (restored_text_index == umax)
 				{
 					progr0.text_index = umax;
-					progr0.update_id = 0;
+					progr0.update_id++;
 					return true;
 				}
 
@@ -191,16 +153,15 @@ scoped_progress_dialog::~scoped_progress_dialog() noexcept
 			}
 
 			return true;
-		}))
+		});
+
+		progr = old;
+
+		if (!ok)
 		{
 			continue;
 		}
 
 		break;
 	}
-
-	if (unmap)
-	{
-		g_progr_text_queue[m_text_index].reset();
-	}
 }

rpcs3/Emu/system_progress.hpp

@@ -12,7 +12,8 @@ struct alignas(16) progress_dialog_string_t
 	struct alignas(16) data_t
 	{
 		usz update_id = 0;
-		usz text_index = umax;
+		u32 text_index = umax;
+		u32 text_count = 0;
 	};
 
 	atomic_t<data_t> data{};
@@ -31,7 +32,7 @@ struct alignas(16) progress_dialog_string_t
 
 	explicit operator bool() const noexcept
 	{
-		return get_string_ptr().operator bool();
+		return data.load().text_count != 0;
 	}
 };
 
@@ -49,7 +50,7 @@ extern atomic_t<bool> g_system_progress_stopping;
 class scoped_progress_dialog final
 {
 private:
-	usz m_text_index = 0;
+	u32 m_text_index = 0;
 
 public:
 	scoped_progress_dialog(std::string text) noexcept;