Skip to content

Fix ProcessNewBlock vs EnforceBestChainLock deadlocks in ActivateBestChain #3492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UdjinM6 merged 4 commits into from
Jun 9, 2020
Merged

Fix ProcessNewBlock vs EnforceBestChainLock deadlocks in ActivateBestChain #3492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a13084
Drop dead code in DoInvalidateBlock
UdjinM6 May 16, 2020
1c9f6da
Let ActivateBestChain skip SyncWithValidationInterfaceQueue when call…
UdjinM6 May 16, 2020
e279acf
Use CL's own scheduler instead of a global one
UdjinM6 May 19, 2020
509bb94
Revert "Let ActivateBestChain skip SyncWithValidationInterfaceQueue w…
UdjinM6 May 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1832,7 +1832,7 @@ bool AppInitMain()
deterministicMNManager.reset();
deterministicMNManager.reset(new CDeterministicMNManager(*evoDb));

llmq::InitLLMQSystem(*evoDb, &scheduler, false, fReset || fReindexChainState);
llmq::InitLLMQSystem(*evoDb, false, fReset || fReindexChainState);

if (fReset) {
pblocktree->WriteReindexing(true);
Expand Down
35 changes: 16 additions & 19 deletions src/llmq/quorums_chainlocks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,17 @@ std::string CChainLockSig::ToString() const
return strprintf("CChainLockSig(nHeight=%d, blockHash=%s)", nHeight, blockHash.ToString());
}

CChainLocksHandler::CChainLocksHandler(CScheduler* _scheduler) :
scheduler(_scheduler)
CChainLocksHandler::CChainLocksHandler()
{
scheduler = new CScheduler();
CScheduler::Function serviceLoop = boost::bind(&CScheduler::serviceQueue, scheduler);
scheduler_thread = new boost::thread(boost::bind(&TraceThread<CScheduler::Function>, "cl-scheduler", serviceLoop));
}

CChainLocksHandler::~CChainLocksHandler()
{
delete scheduler_thread;
delete scheduler;
}

void CChainLocksHandler::Start()
Expand All @@ -56,6 +60,8 @@ void CChainLocksHandler::Start()
void CChainLocksHandler::Stop()
{
quorumSigningManager->UnregisterRecoveredSigsListener(this);
scheduler_thread->interrupt();
scheduler_thread->join();
}

bool CChainLocksHandler::AlreadyHave(const CInv& inv)
Expand Down Expand Up @@ -518,7 +524,7 @@ void CChainLocksHandler::EnforceBestChainLock()
}
LogPrintf("CChainLocksHandler::%s -- CLSIG (%s) invalidates block %s\n",
__func__, clsig.ToString(), jt->second->GetBlockHash().ToString());
DoInvalidateBlock(jt->second, false);
DoInvalidateBlock(jt->second);
}

pindex = pindex->pprev;
Expand Down Expand Up @@ -581,27 +587,18 @@ void CChainLocksHandler::HandleNewRecoveredSig(const llmq::CRecoveredSig& recove
}

// WARNING, do not hold cs while calling this method as we'll otherwise run into a deadlock
void CChainLocksHandler::DoInvalidateBlock(const CBlockIndex* pindex, bool activateBestChain)
void CChainLocksHandler::DoInvalidateBlock(const CBlockIndex* pindex)
{
auto& params = Params();

{
LOCK(cs_main);
LOCK(cs_main);

// get the non-const pointer
CBlockIndex* pindex2 = mapBlockIndex[pindex->GetBlockHash()];
auto& params = Params();

CValidationState state;
if (!InvalidateBlock(state, params, pindex2)) {
LogPrintf("CChainLocksHandler::%s -- InvalidateBlock failed: %s\n", __func__, FormatStateMessage(state));
// This should not have happened and we are in a state were it's not safe to continue anymore
assert(false);
}
}
// get the non-const pointer
CBlockIndex* pindex2 = mapBlockIndex[pindex->GetBlockHash()];

CValidationState state;
if (activateBestChain && !ActivateBestChain(state, params)) {
LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state));
if (!InvalidateBlock(state, params, pindex2)) {
LogPrintf("CChainLocksHandler::%s -- InvalidateBlock failed: %s\n", __func__, FormatStateMessage(state));
// This should not have happened and we are in a state were it's not safe to continue anymore
assert(false);
}
Expand Down
7 changes: 5 additions & 2 deletions src/llmq/quorums_chainlocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include <atomic>
#include <unordered_set>

#include <boost/thread.hpp>

class CBlockIndex;
class CScheduler;

Expand Down Expand Up @@ -52,6 +54,7 @@ class CChainLocksHandler : public CRecoveredSigsListener

private:
CScheduler* scheduler;
boost::thread* scheduler_thread;
CCriticalSection cs;
bool tryLockChainTipScheduled{false};
bool isSporkActive{false};
Expand All @@ -78,7 +81,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
int64_t lastCleanupTime{0};

public:
explicit CChainLocksHandler(CScheduler* _scheduler);
explicit CChainLocksHandler();
~CChainLocksHandler();

void Start();
Expand Down Expand Up @@ -110,7 +113,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
bool InternalHasChainLock(int nHeight, const uint256& blockHash);
bool InternalHasConflictingChainLock(int nHeight, const uint256& blockHash);

void DoInvalidateBlock(const CBlockIndex* pindex, bool activateBestChain);
void DoInvalidateBlock(const CBlockIndex* pindex);

BlockTxs::mapped_type GetBlockTxs(const uint256& blockHash);

Expand Down
5 changes: 2 additions & 3 deletions src/llmq/quorums_init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include <llmq/quorums_signing_shares.h>

#include <dbwrapper.h>
#include <scheduler.h>

namespace llmq
{
Expand All @@ -24,7 +23,7 @@ CBLSWorker* blsWorker;

CDBWrapper* llmqDb;

void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests, bool fWipe)
void InitLLMQSystem(CEvoDB& evoDb, bool unitTests, bool fWipe)
{
llmqDb = new CDBWrapper(unitTests ? "" : (GetDataDir() / "llmq"), 1 << 20, unitTests, fWipe);
blsWorker = new CBLSWorker();
Expand All @@ -35,7 +34,7 @@ void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests, bool f
quorumManager = new CQuorumManager(evoDb, *blsWorker, *quorumDKGSessionManager);
quorumSigSharesManager = new CSigSharesManager();
quorumSigningManager = new CSigningManager(*llmqDb, unitTests);
chainLocksHandler = new CChainLocksHandler(scheduler);
chainLocksHandler = new CChainLocksHandler();
quorumInstantSendManager = new CInstantSendManager(*llmqDb);
}

Expand Down
3 changes: 1 addition & 2 deletions src/llmq/quorums_init.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

class CDBWrapper;
class CEvoDB;
class CScheduler;

namespace llmq
{
Expand All @@ -16,7 +15,7 @@ namespace llmq
static const bool DEFAULT_WATCH_QUORUMS = false;

// Init/destroy LLMQ globals
void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests, bool fWipe = false);
void InitLLMQSystem(CEvoDB& evoDb, bool unitTests, bool fWipe = false);
void DestroyLLMQSystem();

// Manage scheduled tasks, threads, listeners etc.
Expand Down
3 changes: 2 additions & 1 deletion src/test/test_dash.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
connman = g_connman.get();
pblocktree.reset(new CBlockTreeDB(1 << 20, true));
pcoinsdbview.reset(new CCoinsViewDB(1 << 23, true));
llmq::InitLLMQSystem(*evoDb, nullptr, true);
llmq::InitLLMQSystem(*evoDb, true);
pcoinsTip.reset(new CCoinsViewCache(pcoinsdbview.get()));
if (!LoadGenesisBlock(chainparams)) {
throw std::runtime_error("LoadGenesisBlock failed.");
Expand All @@ -117,6 +117,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
TestingSetup::~TestingSetup()
{
llmq::InterruptLLMQSystem();
llmq::StopLLMQSystem();
threadGroup.interrupt_all();
threadGroup.join_all();
GetMainSignals().FlushBackgroundCallbacks();
Expand Down