Skip to content

Commit 693384e

Browse files
committed
qt: Prevent thread/memory leak on exiting RPCConsole
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.
1 parent 47db075 commit 693384e

File tree

4 files changed

+29
-16
lines changed

4 files changed

+29
-16
lines changed

src/qt/bitcoin.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,11 @@ void BitcoinApplication::requestInitialize()
409409

410410
void BitcoinApplication::requestShutdown()
411411
{
412+
// Show a simple window indicating shutdown status
413+
// Do this first as some of the steps may take some time below,
414+
// for example the RPC console may still be executing a command.
415+
ShutdownWindow::showShutdownWindow(window);
416+
412417
qDebug() << __func__ << ": Requesting shutdown";
413418
startThread();
414419
window->hide();
@@ -423,9 +428,6 @@ void BitcoinApplication::requestShutdown()
423428
delete clientModel;
424429
clientModel = 0;
425430

426-
// Show a simple window indicating shutdown status
427-
ShutdownWindow::showShutdownWindow(window);
428-
429431
// Request shutdown from core thread
430432
Q_EMIT requestedShutdown();
431433
}

src/qt/bitcoingui.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,13 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel)
511511
// Disable context menu on tray icon
512512
trayIconMenu->clear();
513513
}
514+
// Propagate cleared model to child objects
515+
rpcConsole->setClientModel(nullptr);
516+
#ifdef ENABLE_WALLET
517+
walletFrame->setClientModel(nullptr);
518+
#endif // ENABLE_WALLET
519+
unitDisplayControl->setOptionsModel(nullptr);
520+
connectionsControl->setClientModel(nullptr);
514521
}
515522
}
516523

@@ -1242,7 +1249,5 @@ void NetworkToggleStatusBarControl::mousePressEvent(QMouseEvent *event)
12421249
/** Lets the control know about the Client Model */
12431250
void NetworkToggleStatusBarControl::setClientModel(ClientModel *_clientModel)
12441251
{
1245-
if (_clientModel) {
1246-
this->clientModel = _clientModel;
1247-
}
1252+
this->clientModel = _clientModel;
12481253
}

src/qt/rpcconsole.cpp

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,6 @@ RPCConsole::RPCConsole(const PlatformStyle *_platformStyle, QWidget *parent) :
382382
// based timer interface
383383
RPCSetTimerInterfaceIfUnset(rpcTimerInterface);
384384

385-
startExecutor();
386385
setTrafficGraphRange(INITIAL_TRAFFIC_GRAPH_MINS);
387386

388387
ui->detailWidget->hide();
@@ -396,7 +395,6 @@ RPCConsole::RPCConsole(const PlatformStyle *_platformStyle, QWidget *parent) :
396395
RPCConsole::~RPCConsole()
397396
{
398397
GUIUtil::saveWindowGeometry("nRPCConsoleWindow", this);
399-
Q_EMIT stopExecutor();
400398
RPCUnsetTimerInterface(rpcTimerInterface);
401399
delete rpcTimerInterface;
402400
delete ui;
@@ -565,6 +563,14 @@ void RPCConsole::setClientModel(ClientModel *model)
565563
autoCompleter = new QCompleter(wordList, this);
566564
ui->lineEdit->setCompleter(autoCompleter);
567565
autoCompleter->popup()->installEventFilter(this);
566+
// Start thread to execute RPC commands.
567+
startExecutor();
568+
}
569+
if (!model) {
570+
// Client model is being set to 0, this means shutdown() is about to be called.
571+
// Make sure we clean up the executor thread
572+
Q_EMIT stopExecutor();
573+
thread.wait();
568574
}
569575
}
570576

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

760766
void RPCConsole::startExecutor()
761767
{
762-
QThread *thread = new QThread;
763768
RPCExecutor *executor = new RPCExecutor();
764-
executor->moveToThread(thread);
769+
executor->moveToThread(&thread);
765770

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

771776
// On stopExecutor signal
772-
// - queue executor for deletion (in execution thread)
773777
// - quit the Qt event loop in the execution thread
774-
connect(this, SIGNAL(stopExecutor()), executor, SLOT(deleteLater()));
775-
connect(this, SIGNAL(stopExecutor()), thread, SLOT(quit()));
776-
// Queue the thread for deletion (in this thread) when it is finished
777-
connect(thread, SIGNAL(finished()), thread, SLOT(deleteLater()));
778+
connect(this, SIGNAL(stopExecutor()), &thread, SLOT(quit()));
779+
// - queue executor for deletion (in execution thread)
780+
connect(&thread, SIGNAL(finished()), executor, SLOT(deleteLater()), Qt::DirectConnection);
781+
connect(&thread, SIGNAL(finished()), this, SLOT(test()), Qt::DirectConnection);
778782

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

784788
void RPCConsole::on_tabWidget_currentChanged(int index)

src/qt/rpcconsole.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include <QWidget>
1414
#include <QCompleter>
15+
#include <QThread>
1516

1617
class ClientModel;
1718
class PlatformStyle;
@@ -148,6 +149,7 @@ public Q_SLOTS:
148149
QMenu *banTableContextMenu;
149150
int consoleFontSize;
150151
QCompleter *autoCompleter;
152+
QThread thread;
151153

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

0 commit comments

Comments
 (0)