sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-26 04:22:55 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix: double lock of deterministicMNManager->cs (#5637)

Browse Source 
## Issue being fixed or feature implemented

fix: double lock of deterministicMNManager->cs

Logs:
```
 node1 2023-10-21T16:46:03.990302Z (mocktime: 2014-12-04T17:33:19Z) [httpworker.1] DOUBLE LOCK DETECTED
 node1 2023-10-21T16:46:03.990322Z (mocktime: 2014-12-04T17:33:19Z) [httpworker.1] Lock order:
 node1 2023-10-21T16:46:03.990339Z (mocktime: 2014-12-04T17:33:19Z) [httpworker.1]  'cs_main' in miner.cpp:129 (in thread 'httpworker.1')
 node1 2023-10-21T16:46:03.990353Z (mocktime: 2014-12-04T17:33:19Z) [httpworker.1]  'm_mempool.cs' in miner.cpp:129 (in thread 'httpworker.1')
 node1 2023-10-21T16:46:03.990366Z (mocktime: 2014-12-04T17:33:19Z) [httpworker.1]  (*) 'deterministicMNManager->cs' in evo/cbtx.cpp:114 (in thread 'httpworker.1')
 node1 2023-10-21T16:46:03.990439Z (mocktime: 2014-12-04T17:33:19Z) [httpworker.1]  (*) 'cs' in ./evo/deterministicmns.h:614 (in thread 'httpworker.1')
 node1 2023-10-21T16:46:04.003619Z (mocktime: 2014-12-04T17:33:19Z) [httpworker.1] Posix Signal: Aborted
                                   No debug information available for stacktrace. You should add debug information and then run:
                                   dashd -printcrashinfo=bvcgc43iinzgc43ijfxgm3ybaacwiyltnbsbkudponuxqictnftw4ylmhiqecytpoj2gkzaphcbzaaaaaaaaayeurhimekiaaav6ldwqyiuqaafwsoe5bqrjaaahz6eh2dbcsaaakhhzaaaaaaaaanreseaaaaaaacgguliaaaaaaadyauwqaaaaaaacp3daaaaaaaaamxigcaaaaaaablpulyaaaaaaadovy3yaaaaaaahj7vbaaaaaaaadnbsdaaaaaaaaaa======
```

Part of backtrace:
```
    #9  UniqueLock<AnnotatedMixin<std::mutex>, std::unique_lock<std::mutex> >::UniqueLock (fTry=false, nLine=615, pszFile=0x55a9f71a3710 "./evo/deterministicmns.h",
        pszName=0x55a9f719caff "cs", mutexIn=..., this=0x7f7e1e71b250) at ./sync.h:164
    #10 CDeterministicMNManager::GetListForBlock (this=0x55a9f84d06b0, pindex=0x7f7db03621c0) at ./evo/deterministicmns.h:615
    #11 0x000055a9f6612258 in llmq::utils::ComputeQuorumMembersByQuarterRotation (pCycleQuorumBaseBlockIndex=0x7f7db03a6930, llmqParams=...) at /usr/include/c++/12/bits/unique_ptr.h:191
    #12 llmq::utils::GetAllQuorumMembers (llmqType=<optimized out>, pQuorumBaseBlockIndex=0x7f7db0359bc0, reset_cache=reset_cache@entry=false) at llmq/utils.cpp:150
    #13 0x000055a9f694d957 in CDeterministicMNManager::HandleQuorumCommitment (qc=..., pQuorumBaseBlockIndex=<optimized out>, mnList=..., debugLogs=debugLogs@entry=false)
        at evo/deterministicmns.cpp:989
    #14 0x000055a9f695c455 in CDeterministicMNManager::BuildNewListFromBlock (this=<optimized out>, block=..., pindexPrev=pindexPrev@entry=0x7f7db03c1ac0, state=..., view=...,
        mnListRet=..., debugLogs=false) at evo/deterministicmns.cpp:918
    #15 0x000055a9f692e7cd in CalcCbTxMerkleRootMNList (block=..., pindexPrev=pindexPrev@entry=0x7f7db03c1ac0, merkleRootRet=..., state=..., view=...)
        at /usr/include/c++/12/bits/unique_ptr.h:191
    #16 0x000055a9f6a352ed in BlockAssembler::CreateNewBlock (this=this@entry=0x7f7e1e71f0b0, scriptPubKeyIn=...) at ./validation.h:649
    #17 0x000055a9f6771c49 in generateBlocks (chainman=..., mempool=..., evodb=..., llmq_ctx=..., coinbase_script=..., nGenerate=10, nMaxTries=<optimized out>) at rpc/mining.cpp:167
    #18 0x000055a9f677a496 in generatetoaddress (request=...) at /usr/include/c++/12/bits/unique_ptr.h:191
    #19 0x000055a9f671ea02 in CRPCCommand::CRPCCommand(char const*, char const*, UniValue (*)(JSONRPCRequest const&), std::initializer_list<char const*>)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const (__closure=<optimized out>, result=..., request=...) at ./rpc/server.h:120
```



## What was done?
`CDeterministicMNManager::BuildNewListFromBlock` doesn't require `cs`
lock anymore

## How Has This Been Tested?
Run unit/functional tests


## Breaking Changes
N/A

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
This commit is contained in:
Konstantin Akimov 2023-10-23 22:33:47 +07:00 committed by GitHub
parent c51cec606d
commit 343db5ffb5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/evo/cbtx.cpp
View File

@ -111,12 +111,10 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, const
bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev, uint256& merkleRootRet, BlockValidationState& state, const CCoinsViewCache& view) bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev, uint256& merkleRootRet, BlockValidationState& state, const CCoinsViewCache& view)
{ {
LOCK(deterministicMNManager->cs);
try { try {
static int64_t nTimeDMN = 0; static std::atomic<int64_t> nTimeDMN = 0;
static int64_t nTimeSMNL = 0; static std::atomic<int64_t> nTimeSMNL = 0;
static int64_t nTimeMerkle = 0; static std::atomic<int64_t> nTimeMerkle = 0;
int64_t nTime1 = GetTimeMicros(); int64_t nTime1 = GetTimeMicros();
@ -134,10 +132,12 @@ bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev
int64_t nTime3 = GetTimeMicros(); nTimeSMNL += nTime3 - nTime2; int64_t nTime3 = GetTimeMicros(); nTimeSMNL += nTime3 - nTime2;
LogPrint(BCLog::BENCHMARK, " - CSimplifiedMNList: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeSMNL * 0.000001); LogPrint(BCLog::BENCHMARK, " - CSimplifiedMNList: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeSMNL * 0.000001);
static CSimplifiedMNList smlCached; static Mutex cached_mutex;
static uint256 merkleRootCached; static CSimplifiedMNList smlCached GUARDED_BY(cached_mutex);
static bool mutatedCached{false}; static uint256 merkleRootCached GUARDED_BY(cached_mutex);
static bool mutatedCached GUARDED_BY(cached_mutex) {false};
LOCK(cached_mutex);
if (sml == smlCached) { if (sml == smlCached) {
merkleRootRet = merkleRootCached; merkleRootRet = merkleRootCached;
if (mutatedCached) { if (mutatedCached) {

8
src/evo/deterministicmns.cpp
View File

@ -613,8 +613,6 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
int nHeight = pindex->nHeight; int nHeight = pindex->nHeight;
try { try {
LOCK(cs);
if (!BuildNewListFromBlock(block, pindex->pprev, state, view, newList, true)) { if (!BuildNewListFromBlock(block, pindex->pprev, state, view, newList, true)) {
// pass the state returned by the function above // pass the state returned by the function above
return false; return false;
@ -626,6 +624,8 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
newList.SetBlockHash(pindex->GetBlockHash()); newList.SetBlockHash(pindex->GetBlockHash());
LOCK(cs);
oldList = GetListForBlockInternal(pindex->pprev); oldList = GetListForBlockInternal(pindex->pprev);
diff = oldList.BuildDiff(newList); diff = oldList.BuildDiff(newList);
@ -708,11 +708,9 @@ void CDeterministicMNManager::UpdatedBlockTip(const CBlockIndex* pindex)
bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, BlockValidationState& state, const CCoinsViewCache& view, CDeterministicMNList& mnListRet, bool debugLogs) bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, BlockValidationState& state, const CCoinsViewCache& view, CDeterministicMNList& mnListRet, bool debugLogs)
{ {
AssertLockHeld(cs);
int nHeight = pindexPrev->nHeight + 1; int nHeight = pindexPrev->nHeight + 1;
CDeterministicMNList oldList = GetListForBlockInternal(pindexPrev); CDeterministicMNList oldList = GetListForBlock(pindexPrev);
CDeterministicMNList newList = oldList; CDeterministicMNList newList = oldList;
newList.SetBlockHash(uint256()); // we can't know the final block hash, so better not return a (invalid) block hash newList.SetBlockHash(uint256()); // we can't know the final block hash, so better not return a (invalid) block hash
newList.SetHeight(nHeight); newList.SetHeight(nHeight);

6
src/evo/deterministicmns.h
View File

@ -574,10 +574,8 @@ class CDeterministicMNManager
static constexpr int DISK_SNAPSHOTS = llmq_max_blocks() / DISK_SNAPSHOT_PERIOD + 1; static constexpr int DISK_SNAPSHOTS = llmq_max_blocks() / DISK_SNAPSHOT_PERIOD + 1;
static constexpr int LIST_DIFFS_CACHE_SIZE = DISK_SNAPSHOT_PERIOD * DISK_SNAPSHOTS; static constexpr int LIST_DIFFS_CACHE_SIZE = DISK_SNAPSHOT_PERIOD * DISK_SNAPSHOTS;
public:
Mutex cs;
private: private:
Mutex cs;
Mutex cs_cleanup; Mutex cs_cleanup;
// We have performed CleanupCache() on this height. // We have performed CleanupCache() on this height.
int did_cleanup GUARDED_BY(cs_cleanup) {0}; int did_cleanup GUARDED_BY(cs_cleanup) {0};
@ -607,7 +605,7 @@ public:
// the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet) // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet)
bool BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, BlockValidationState& state, const CCoinsViewCache& view, bool BuildNewListFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, BlockValidationState& state, const CCoinsViewCache& view,
CDeterministicMNList& mnListRet, bool debugLogs) EXCLUSIVE_LOCKS_REQUIRED(cs); CDeterministicMNList& mnListRet, bool debugLogs) LOCKS_EXCLUDED(cs);
static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, const CBlockIndex* pQuorumBaseBlockIndex, CDeterministicMNList& mnList, bool debugLogs); static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, const CBlockIndex* pQuorumBaseBlockIndex, CDeterministicMNList& mnList, bool debugLogs);
CDeterministicMNList GetListForBlock(const CBlockIndex* pindex) LOCKS_EXCLUDED(cs) { CDeterministicMNList GetListForBlock(const CBlockIndex* pindex) LOCKS_EXCLUDED(cs) {