highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 14:38:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #18338: Fix wallet unload race condition

Browse Source 
41b0baf43c gui: Handle WalletModel::unload asynchronous (João Barbosa)
ab31b9d6fe Fix wallet unload race condition (Russell Yanofsky)

Pull request description:

  This PR consists in two fixes. The first fixes a concurrency issues with `boost::signals2`. The second fixes a wallet model destruction while it's being used.

  From boost signal documentation at https://www.boost.org/doc/libs/1_72_0/doc/html/signals2/thread-safety.html:

  > When a signal is invoked by calling signal::operator(), the invocation first acquires a lock on the signal's mutex. Then it obtains a handle to the signal's slot list and combiner. Next it releases the signal's mutex, before invoking the combiner to iterate through the slot list.

  This means that `UnregisterValidationInterface` doesn't prevent more calls to that interface. The fix consists in capturing the `shared_ptr<CValidationInterface>` in each internal slot.

  The GUI bug is fixed by using a `Qt::QueuedConnection` in the `WalletModel::unload` connection.

ACKs for top commit:
  ryanofsky:
    Code review ACK 41b0baf43c. Only change is moving assert as suggested
  hebasto:
    ACK 41b0baf43c, tested on Linux Mint 19.3.

Tree-SHA512: 4f712d8de65bc1214411831250de5dc0a9fd505fb84da5baf9f2cc4d551bc3abffc061616f00afe43dba7525af2cd96c9b54aeead9383145e3b8801f25d85f50
This commit is contained in:
Wladimir J. van der Laan
2020-03-31 14:25:04 +02:00
parent f2880e21ef 41b0baf43c
commit 9a2b5f22c1
10 changed files with 64 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
src/interfaces/chain.cpp
View File

@@ -148,22 +148,12 @@ class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
using UniqueLock::UniqueLock;
};
class NotificationsHandlerImpl : public Handler, CValidationInterface
class NotificationsProxy : public CValidationInterface
{
public:
explicit NotificationsHandlerImpl(Chain& chain, Chain::Notifications& notifications)
: m_chain(chain), m_notifications(&notifications)
{
RegisterValidationInterface(this);
}
~NotificationsHandlerImpl() override { disconnect(); }
void disconnect() override
{
if (m_notifications) {
m_notifications = nullptr;
UnregisterValidationInterface(this);
}
}
explicit NotificationsProxy(std::shared_ptr<Chain::Notifications> notifications)
: m_notifications(std::move(notifications)) {}
virtual ~NotificationsProxy() = default;
void TransactionAddedToMempool(const CTransactionRef& tx) override
{
m_notifications->transactionAddedToMempool(tx);
@@ -185,8 +175,26 @@ public:
m_notifications->updatedBlockTip();
}
void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->chainStateFlushed(locator); }
Chain& m_chain;
Chain::Notifications* m_notifications;
std::shared_ptr<Chain::Notifications> m_notifications;
};
class NotificationsHandlerImpl : public Handler
{
public:
explicit NotificationsHandlerImpl(std::shared_ptr<Chain::Notifications> notifications)
: m_proxy(std::make_shared<NotificationsProxy>(std::move(notifications)))
{
RegisterSharedValidationInterface(m_proxy);
}
~NotificationsHandlerImpl() override { disconnect(); }
void disconnect() override
{
if (m_proxy) {
UnregisterSharedValidationInterface(m_proxy);
m_proxy.reset();
}
}
std::shared_ptr<NotificationsProxy> m_proxy;
};
class RpcHandlerImpl : public Handler
@@ -342,9 +350,9 @@ public:
{
::uiInterface.ShowProgress(title, progress, resume_possible);
}
std::unique_ptr<Handler> handleNotifications(Notifications& notifications) override
std::unique_ptr<Handler> handleNotifications(std::shared_ptr<Notifications> notifications) override
{
return MakeUnique<NotificationsHandlerImpl>(*this, notifications);
return MakeUnique<NotificationsHandlerImpl>(std::move(notifications));
}
void waitForNotificationsIfTipChanged(const uint256& old_tip) override
{

2
src/interfaces/chain.h
View File

@@ -229,7 +229,7 @@ public:
};
//! Register handler for notifications.
virtual std::unique_ptr<Handler> handleNotifications(Notifications& notifications) = 0;
virtual std::unique_ptr<Handler> handleNotifications(std::shared_ptr<Notifications> notifications) = 0;
//! Wait for pending notifications to be processed unless block hash points to the current
//! chain tip.