From 60e190ceb3563a8102d42fdfcbefccdd1b53e812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 29 Jan 2019 00:06:17 +0000 Subject: [PATCH 1/4] gui: Fix WalletController deletion The wallet controller instanced must be deleted after the window instance since it is used there. --- src/qt/bitcoin.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index ca26131b957..0f06bbaaa6a 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -218,6 +218,8 @@ BitcoinApplication::~BitcoinApplication() #ifdef ENABLE_WALLET delete paymentServer; paymentServer = nullptr; + delete m_wallet_controller; + m_wallet_controller = nullptr; #endif delete optionsModel; optionsModel = nullptr; @@ -310,10 +312,6 @@ void BitcoinApplication::requestShutdown() window->setClientModel(nullptr); pollShutdownTimer->stop(); -#ifdef ENABLE_WALLET - delete m_wallet_controller; - m_wallet_controller = nullptr; -#endif delete clientModel; clientModel = nullptr; From 07b9aadcfc7cc72be9df344dd5715cf8fc78f0ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 29 Jan 2019 00:08:20 +0000 Subject: [PATCH 2/4] gui: Expose BitcoinGUI::unsubscribeFromCoreSignals Move only change that makes unsubscribeFromCoreSignals public. It must be called if the event loop is not running otherwise core signals handlers can deadlock. --- src/qt/bitcoingui.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index f1b76a6b641..c31cefe603f 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -95,6 +95,9 @@ public: */ bool hasTrayIcon() const { return trayIcon; } + /** Disconnect core signals from GUI client */ + void unsubscribeFromCoreSignals(); + protected: void changeEvent(QEvent *e); void closeEvent(QCloseEvent *event); @@ -184,8 +187,6 @@ private: /** Connect core signals to GUI client */ void subscribeToCoreSignals(); - /** Disconnect core signals from GUI client */ - void unsubscribeFromCoreSignals(); /** Update UI with latest network info from model. */ void updateNetworkState(); From fd6d499bdacfa29f25b0f675375e3feaced08667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 29 Jan 2019 00:08:48 +0000 Subject: [PATCH 3/4] gui: Fix m_node.startShutdown() order This change forwards the shutdown request on the GUI (close the application for instace) to the node as soon as possible. This way the GUI doesn't have to wait for long operations to complete (rescan the wallet for instance), instead those operations detect the shutdown request and abort/interrupt. --- src/qt/bitcoin.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 0f06bbaaa6a..85d79ee26c3 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -309,14 +309,20 @@ void BitcoinApplication::requestShutdown() qDebug() << __func__ << ": Requesting shutdown"; startThread(); window->hide(); + // Must disconnect node signals otherwise current thread can deadlock since + // no event loop is running. + window->unsubscribeFromCoreSignals(); + // Request node shutdown, which can interrupt long operations, like + // rescanning a wallet. + m_node.startShutdown(); + // Unsetting the client model can cause the current thread to wait for node + // to complete an operation, like wait for a RPC execution to complate. window->setClientModel(nullptr); pollShutdownTimer->stop(); delete clientModel; clientModel = nullptr; - m_node.startShutdown(); - // Request shutdown from core thread Q_EMIT requestedShutdown(); } From 0dd6a8c12489ea4428b398a2328dde5d1a9fe39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 29 Jan 2019 01:08:30 +0000 Subject: [PATCH 4/4] Check m_internals in UnregisterValidationInterface When a wallet is created it is registered in the validation interface (in CWallet::CreateWalletFromFile) but it is not immediately added to the wallets list. If a shutdown is requested before AddWallet (case more evident when -rescan is set) then m_internals can be released (in Shutdown -> UnregisterBackgroundSignalScheduler) before the wallet and then ReleaseWallet would call UnregisterValidationInterface with m_internals already released. --- src/validationinterface.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 2e13bef19eb..70c274d20e3 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -107,7 +107,9 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { } void UnregisterValidationInterface(CValidationInterface* pwalletIn) { - g_signals.m_internals->m_connMainSignals.erase(pwalletIn); + if (g_signals.m_internals) { + g_signals.m_internals->m_connMainSignals.erase(pwalletIn); + } } void UnregisterAllValidationInterfaces() {