From 1915008d0043a0f6a672753987139fa72971160e Mon Sep 17 00:00:00 2001 From: Akash Mozumdar Date: Wed, 31 Oct 2018 01:20:44 -0400 Subject: [PATCH] using shared_ptr to improve thread safety --- GUI/host/host.cc | 24 +++++++++--------------- GUI/host/host.h | 7 ++++--- GUI/host/textthread.cc | 2 +- GUI/mainwindow.cpp | 23 ++++++++++++----------- GUI/mainwindow.h | 14 ++++++++------ include/common.h | 2 ++ 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/GUI/host/host.cc b/GUI/host/host.cc index 234e42b..3071b4e 100644 --- a/GUI/host/host.cc +++ b/GUI/host/host.cc @@ -21,7 +21,7 @@ namespace ThreadEventCallback OnCreate, OnRemove; ProcessEventCallback OnAttach, OnDetach; - std::unordered_map textThreadsByParams; + std::unordered_map> textThreadsByParams; std::unordered_map 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(tp)); } textThreadsByParams[tp]->AddText(text, len); } @@ -43,15 +43,12 @@ namespace void RemoveThreads(std::function removeIf) { LOCK(hostMutex); - std::vector 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(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 GetThread(ThreadParam tp) { LOCK(hostMutex); return textThreadsByParams[tp]; diff --git a/GUI/host/host.h b/GUI/host/host.h index b27d1a8..f933b7a 100644 --- a/GUI/host/host.h +++ b/GUI/host/host.h @@ -8,7 +8,7 @@ #include "textthread.h" typedef std::function ProcessEventCallback; -typedef std::function ThreadEventCallback; +typedef std::function)> 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 GetThread(ThreadParam tp); void AddConsoleOutput(std::wstring text); } diff --git a/GUI/host/textthread.cc b/GUI/host/textthread.cc index 895d99d..095030c 100644 --- a/GUI/host/textthread.cc +++ b/GUI/host/textthread.cc @@ -8,7 +8,7 @@ #include #include -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() { diff --git a/GUI/mainwindow.cpp b/GUI/mainwindow.cpp index 7d689ba..24ed687 100644 --- a/GUI/mainwindow.cpp +++ b/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>(); + 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 thread) { emit SigAddThread(thread); }, + [&](std::shared_ptr 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 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 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; diff --git a/GUI/mainwindow.h b/GUI/mainwindow.h index a57d672..91a3840 100644 --- a/GUI/mainwindow.h +++ b/GUI/mainwindow.h @@ -13,6 +13,8 @@ namespace Ui class MainWindow; } +Q_DECLARE_METATYPE(std::shared_ptr); + 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); + void SigRemoveThread(std::shared_ptr); + 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 thread); + void RemoveThread(std::shared_ptr 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); diff --git a/include/common.h b/include/common.h index 2872a93..11a649d 100644 --- a/include/common.h +++ b/include/common.h @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include #include