using shared_ptr to improve thread safety

GUI/host/host.cc

@@ -21,7 +21,7 @@ namespace
 	ThreadEventCallback OnCreate, OnRemove;
 	ProcessEventCallback OnAttach, OnDetach;
 
-	std::unordered_map<ThreadParam, TextThread*> textThreadsByParams;
+	std::unordered_map<ThreadParam, std::shared_ptr<TextThread>> textThreadsByParams;
 	std::unordered_map<DWORD, ProcessRecord> processRecordsByIds;
 
 	std::recursive_mutex hostMutex;
@@ -35,7 +35,7 @@ namespace
 		if (textThreadsByParams[tp] == nullptr)
 		{
 			if (textThreadsByParams.size() > MAX_THREAD_COUNT) return Host::AddConsoleOutput(L"too many text threads: can't create more");
-			OnCreate(textThreadsByParams[tp] = new TextThread(tp));
+			OnCreate(textThreadsByParams[tp] = std::make_shared<TextThread>(tp));
 		}
 		textThreadsByParams[tp]->AddText(text, len);
 	}
@@ -43,15 +43,12 @@ namespace
 	void RemoveThreads(std::function<bool(ThreadParam)> removeIf)
 	{
 		LOCK(hostMutex);
-		std::vector<ThreadParam> removedThreads;
-		for (auto[tp, thread] : textThreadsByParams)
-			if (removeIf(tp))
+		for (auto it = textThreadsByParams.begin(); it != textThreadsByParams.end();)
+			if (auto curr = it++; removeIf(curr->first))
 			{
-				OnRemove(thread);
-				//delete i.second; // Artikash 7/24/2018: FIXME: Qt GUI updates on another thread, so I can't delete this yet.
-				removedThreads.push_back(tp);
+				OnRemove(curr->second);
+				textThreadsByParams.erase(curr->first);
 			}
-		for (auto thread : removedThreads) textThreadsByParams.erase(thread);
 	}
 
 	void RegisterProcess(DWORD pid, HANDLE hostPipe)
@@ -143,7 +140,7 @@ namespace Host
 	void Start(ProcessEventCallback onAttach, ProcessEventCallback onDetach, ThreadEventCallback onCreate, ThreadEventCallback onRemove, TextThread::OutputCallback output)
 	{
 		OnAttach = onAttach; OnDetach = onDetach; OnCreate = onCreate; OnRemove = onRemove; TextThread::Output = output;
-		OnCreate(textThreadsByParams[CONSOLE] = new TextThread(CONSOLE));
+		OnCreate(textThreadsByParams[CONSOLE] = std::make_shared<TextThread>(CONSOLE));
 		StartPipe();
 	}
 
@@ -152,9 +149,8 @@ namespace Host
 		// Artikash 7/25/2018: This is only called when Textractor is closed, at which point Windows should free everything itself...right?
 #ifdef _DEBUG // Check memory leaks
 		LOCK(hostMutex);
-		OnRemove = [](TextThread* textThread) { delete textThread; };
 		for (auto[pid, pr] : processRecordsByIds) UnregisterProcess(pid);
-		delete textThreadsByParams[CONSOLE];
+		textThreadsByParams.clear();
 #endif
 	}
 
@@ -239,8 +235,6 @@ namespace Host
 		return ret;
 	}
 
-	HookParam GetHookParam(ThreadParam tp) { return GetHookParam(tp.pid, tp.hook); }
-
 	std::wstring GetHookName(DWORD pid, uint64_t addr)
 	{
 		if (pid == 0) return L"Console";
@@ -260,7 +254,7 @@ namespace Host
 		return StringToWideString(buffer, CP_UTF8);
 	}
 
-	TextThread* GetThread(ThreadParam tp)
+	std::shared_ptr<TextThread> GetThread(ThreadParam tp)
 	{
 		LOCK(hostMutex);
 		return textThreadsByParams[tp];

GUI/host/host.h

@@ -8,7 +8,7 @@
 #include "textthread.h"
 
 typedef std::function<void(DWORD)> ProcessEventCallback;
-typedef std::function<void(TextThread*)> ThreadEventCallback;
+typedef std::function<void(std::shared_ptr<TextThread>)> ThreadEventCallback;
 
 namespace Host
 {
@@ -22,10 +22,11 @@ namespace Host
 	void RemoveHook(DWORD pid, uint64_t addr);
 
 	HookParam GetHookParam(DWORD pid, uint64_t addr);
-	HookParam GetHookParam(ThreadParam tp);
+	inline HookParam GetHookParam(ThreadParam tp) { return GetHookParam(tp.pid, tp.hook); }
 	std::wstring GetHookName(DWORD pid, uint64_t addr);
+	inline std::wstring GetHookName(ThreadParam tp) { return GetHookName(tp.pid, tp.hook); }
 
-	TextThread* GetThread(ThreadParam tp);
+	std::shared_ptr<TextThread> GetThread(ThreadParam tp);
 	void AddConsoleOutput(std::wstring text);
 }
 

GUI/host/textthread.cc

@@ -8,7 +8,7 @@
 #include <regex>
 #include <algorithm>
 
-TextThread::TextThread(ThreadParam tp) : handle(threadCounter++), name(Host::GetHookName(tp.pid, tp.hook)), tp(tp), hp(Host::GetHookParam(tp)) {}
+TextThread::TextThread(ThreadParam tp) : handle(threadCounter++), name(Host::GetHookName(tp)), tp(tp), hp(Host::GetHookParam(tp)) {}
 
 TextThread::~TextThread()
 {

GUI/mainwindow.cpp

@@ -35,6 +35,8 @@ MainWindow::MainWindow(QWidget *parent) :
 	if (settings.contains("Flush_Delay")) TextThread::flushDelay = settings.value("Flush_Delay").toInt();
 	if (settings.contains("Max_Buffer_Size")) TextThread::maxBufferSize = settings.value("Max_Buffer_Size").toInt();
 
+	qRegisterMetaType<std::shared_ptr<TextThread>>();
+
 	connect(this, &MainWindow::SigAddProcess, this, &MainWindow::AddProcess);
 	connect(this, &MainWindow::SigRemoveProcess, this, &MainWindow::RemoveProcess);
 	connect(this, &MainWindow::SigAddThread, this, &MainWindow::AddThread);
@@ -44,8 +46,8 @@ MainWindow::MainWindow(QWidget *parent) :
 	Host::Start(
 		[&](DWORD processId) { emit SigAddProcess(processId); },
 		[&](DWORD processId) { emit SigRemoveProcess(processId); },
-		[&](TextThread* thread) { emit SigAddThread(thread); },
-		[&](TextThread* thread) { emit SigRemoveThread(thread); },
+		[&](std::shared_ptr<TextThread> thread) { emit SigAddThread(thread); },
+		[&](std::shared_ptr<TextThread> thread) { emit SigRemoveThread(thread); },
 		[&](TextThread* thread, std::wstring& output) { return ProcessThreadOutput(thread, output); }
 	);
 	Host::AddConsoleOutput(L"Textractor beta v3.3.2 by Artikash\r\nSource code and more information available under GPLv3 at https://github.com/Artikash/Textractor");
@@ -82,32 +84,31 @@ void MainWindow::RemoveProcess(unsigned processId)
 	processCombo->removeItem(processCombo->findText(QString::number(processId, 16).toUpper() + ":", Qt::MatchStartsWith));
 }
 
-void MainWindow::AddThread(TextThread* thread)
+void MainWindow::AddThread(std::shared_ptr<TextThread> thread)
 {
 	ttCombo->addItem(
-		TextThreadString(thread) +
+		TextThreadString(thread.get()) +
 		QString::fromStdWString(thread->name) +
 		" (" +
-		GenerateCode(Host::GetHookParam(thread->tp), thread->tp.pid) +
+		GenerateCode(thread->hp, thread->tp.pid) +
 		")"
 	);
 }
 
-void MainWindow::RemoveThread(TextThread* thread)
+void MainWindow::RemoveThread(std::shared_ptr<TextThread> thread)
 {
-	int threadIndex = ttCombo->findText(TextThreadString(thread), Qt::MatchStartsWith);
+	int threadIndex = ttCombo->findText(TextThreadString(thread.get()), Qt::MatchStartsWith);
 	if (threadIndex == ttCombo->currentIndex())
 	{
 		ttCombo->setCurrentIndex(0);
 		on_ttCombo_activated(0);
 	}
 	ttCombo->removeItem(threadIndex);
-	delete thread;
 }
 
-void MainWindow::ThreadOutput(TextThread* thread, QString output)
+void MainWindow::ThreadOutput(QString threadString, QString output)
 {
-	if (ttCombo->currentText().startsWith(TextThreadString(thread)))
+	if (ttCombo->currentText().startsWith(threadString))
 	{
 		textOutput->moveCursor(QTextCursor::End);
 		textOutput->insertPlainText(output);
@@ -120,7 +121,7 @@ bool MainWindow::ProcessThreadOutput(TextThread* thread, std::wstring& output)
 	if (Extension::DispatchSentence(output, GetInfoForExtensions(thread)))
 	{
 		output += L"\r\n";
-		emit SigThreadOutput(thread, QString::fromStdWString(output));
+		emit SigThreadOutput(TextThreadString(thread), QString::fromStdWString(output));
 		return true;
 	}
 	return false;

GUI/mainwindow.h

@@ -13,6 +13,8 @@ namespace Ui
 	class MainWindow;
 }
 
+Q_DECLARE_METATYPE(std::shared_ptr<TextThread>);
+
 class MainWindow : public QMainWindow
 {
 	Q_OBJECT
@@ -24,16 +26,16 @@ public:
 signals:
 	void SigAddProcess(unsigned processId);
 	void SigRemoveProcess(unsigned processId);
-	void SigAddThread(TextThread* thread);
-	void SigRemoveThread(TextThread* thread);
-	void SigThreadOutput(TextThread* thread, QString output);
+	void SigAddThread(std::shared_ptr<TextThread>);
+	void SigRemoveThread(std::shared_ptr<TextThread>);
+	void SigThreadOutput(QString threadString, QString output);
 
 private slots:
 	void AddProcess(unsigned processId);
 	void RemoveProcess(unsigned processId);
-	void AddThread(TextThread* thread);
-	void RemoveThread(TextThread* thread);
-	void ThreadOutput(TextThread* thread, QString output);
+	void AddThread(std::shared_ptr<TextThread> thread);
+	void RemoveThread(std::shared_ptr<TextThread> thread);
+	void ThreadOutput(QString threadString, QString output); // this function doesn't take TextThread* because it might be destroyed on pipe thread
 	void on_attachButton_clicked();
 	void on_detachButton_clicked();
 	void on_ttCombo_activated(int index);

include/common.h

@@ -6,6 +6,8 @@
 #include <vector>
 #include <unordered_map>
 #include <functional>
+#include <algorithm>
+#include <memory>
 #include <optional>
 #include <thread>
 #include <mutex>