Skip to content

Commit

Permalink
qt: Prevent thread/memory leak on exiting RPCConsole
Browse files Browse the repository at this point in the history
Make ownership of the QThread object clear, so that the RPCConsole
can wait for the executor thread to quit before shutdown is called. This
increases overall thread safety, and prevents some objects from leaking
on exit.
  • Loading branch information
laanwj committed Nov 23, 2016
1 parent 47db075 commit 693384e
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 16 deletions.
8 changes: 5 additions & 3 deletions src/qt/bitcoin.cpp
Expand Up @@ -409,6 +409,11 @@ void BitcoinApplication::requestInitialize()

void BitcoinApplication::requestShutdown()
{
// Show a simple window indicating shutdown status
// Do this first as some of the steps may take some time below,
// for example the RPC console may still be executing a command.
ShutdownWindow::showShutdownWindow(window);

qDebug() << __func__ << ": Requesting shutdown";
startThread();
window->hide();
Expand All @@ -423,9 +428,6 @@ void BitcoinApplication::requestShutdown()
delete clientModel;
clientModel = 0;

// Show a simple window indicating shutdown status
ShutdownWindow::showShutdownWindow(window);

// Request shutdown from core thread
Q_EMIT requestedShutdown();
}
Expand Down
11 changes: 8 additions & 3 deletions src/qt/bitcoingui.cpp
Expand Up @@ -511,6 +511,13 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel)
// Disable context menu on tray icon
trayIconMenu->clear();
}
// Propagate cleared model to child objects
rpcConsole->setClientModel(nullptr);
#ifdef ENABLE_WALLET
walletFrame->setClientModel(nullptr);
#endif // ENABLE_WALLET
unitDisplayControl->setOptionsModel(nullptr);
connectionsControl->setClientModel(nullptr);
}
}

Expand Down Expand Up @@ -1242,7 +1249,5 @@ void NetworkToggleStatusBarControl::mousePressEvent(QMouseEvent *event)
/** Lets the control know about the Client Model */
void NetworkToggleStatusBarControl::setClientModel(ClientModel *_clientModel)
{
if (_clientModel) {
this->clientModel = _clientModel;
}
this->clientModel = _clientModel;
}
24 changes: 14 additions & 10 deletions src/qt/rpcconsole.cpp
Expand Up @@ -382,7 +382,6 @@ RPCConsole::RPCConsole(const PlatformStyle *_platformStyle, QWidget *parent) :
// based timer interface
RPCSetTimerInterfaceIfUnset(rpcTimerInterface);

startExecutor();
setTrafficGraphRange(INITIAL_TRAFFIC_GRAPH_MINS);

ui->detailWidget->hide();
Expand All @@ -396,7 +395,6 @@ RPCConsole::RPCConsole(const PlatformStyle *_platformStyle, QWidget *parent) :
RPCConsole::~RPCConsole()
{
GUIUtil::saveWindowGeometry("nRPCConsoleWindow", this);
Q_EMIT stopExecutor();
RPCUnsetTimerInterface(rpcTimerInterface);
delete rpcTimerInterface;
delete ui;
Expand Down Expand Up @@ -565,6 +563,14 @@ void RPCConsole::setClientModel(ClientModel *model)
autoCompleter = new QCompleter(wordList, this);
ui->lineEdit->setCompleter(autoCompleter);
autoCompleter->popup()->installEventFilter(this);
// Start thread to execute RPC commands.
startExecutor();
}
if (!model) {
// Client model is being set to 0, this means shutdown() is about to be called.
// Make sure we clean up the executor thread
Q_EMIT stopExecutor();
thread.wait();
}
}

Expand Down Expand Up @@ -759,26 +765,24 @@ void RPCConsole::browseHistory(int offset)

void RPCConsole::startExecutor()
{
QThread *thread = new QThread;
RPCExecutor *executor = new RPCExecutor();
executor->moveToThread(thread);
executor->moveToThread(&thread);

// Replies from executor object must go to this object
connect(executor, SIGNAL(reply(int,QString)), this, SLOT(message(int,QString)));
// Requests from this object must go to executor
connect(this, SIGNAL(cmdRequest(QString)), executor, SLOT(request(QString)));

// On stopExecutor signal
// - queue executor for deletion (in execution thread)
// - quit the Qt event loop in the execution thread
connect(this, SIGNAL(stopExecutor()), executor, SLOT(deleteLater()));
connect(this, SIGNAL(stopExecutor()), thread, SLOT(quit()));
// Queue the thread for deletion (in this thread) when it is finished
connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater()));
connect(this, SIGNAL(stopExecutor()), &thread, SLOT(quit()));
// - queue executor for deletion (in execution thread)
connect(&thread, SIGNAL(finished()), executor, SLOT(deleteLater()), Qt::DirectConnection);
connect(&thread, SIGNAL(finished()), this, SLOT(test()), Qt::DirectConnection);

// Default implementation of QThread::run() simply spins up an event loop in the thread,
// which is what we want.
thread->start();
thread.start();
}

void RPCConsole::on_tabWidget_currentChanged(int index)
Expand Down
2 changes: 2 additions & 0 deletions src/qt/rpcconsole.h
Expand Up @@ -12,6 +12,7 @@

#include <QWidget>
#include <QCompleter>
#include <QThread>

class ClientModel;
class PlatformStyle;
Expand Down Expand Up @@ -148,6 +149,7 @@ public Q_SLOTS:
QMenu *banTableContextMenu;
int consoleFontSize;
QCompleter *autoCompleter;
QThread thread;

/** Update UI with latest network info from model. */
void updateNetworkState();
Expand Down

0 comments on commit 693384e

Please sign in to comment.