Skip to content

Commit df33e74

Browse files
Merge dashpay#6006: fix: resolve potential deadlocks in CJ
b2910fb fix: resolve potential deadlocks in CJ (UdjinM6) Pull request description: ## Issue being fixed or feature implemented ``` POTENTIAL DEADLOCK DETECTED Previous lock order was: (2) 'cs_wallet' in wallet/wallet.cpp:3826 (in thread 'qt-init') (2) 'pwallet->cs_wallet' in wallet/walletdb.cpp:705 (in thread 'qt-init') (1) 'cs_KeyStore' in wallet/scriptpubkeyman.cpp:971 (in thread 'qt-init') Current lock order is: 'cs_deqsessions' in coinjoin/client.cpp:261 (in thread 'main') (1) 'cs_KeyStore' in wallet/scriptpubkeyman.cpp:1522 (in thread 'main') (2) 'cs_wallet' in wallet/wallet.cpp:1629 (in thread 'main') ``` This one is for `ResetPool()`. ## What was done? Lock `cs_wallet` when calling `keyHolderStorage.ReturnAll()` in some places* to ensure the right order of locks. (*In other places `cs_wallet` is held already, no need to double lock). ## How Has This Been Tested? Mixing ## 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 ACKs for top commit: PastaPastaPasta: utACK b2910fb Tree-SHA512: 8be98df021f7683cd496ebe095dd7b32ebea76c7f9c7c085af3bc0a6901d9cfd4d4624e20a2eee1f3b0d69fd711f8fceb9a91c386b9bf02475632a23b3a0f09a
2 parents d44b0d5 + b2910fb commit df33e74

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

src/coinjoin/client.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ void CCoinJoinClientSession::ResetPool()
249249
{
250250
txMyCollateral = CMutableTransaction();
251251
UnlockCoins();
252-
keyHolderStorage.ReturnAll();
252+
WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
253253
WITH_LOCK(cs_coinjoin, SetNull());
254254
}
255255

@@ -410,7 +410,7 @@ bool CCoinJoinClientSession::CheckTimeout()
410410

411411
SetState(POOL_STATE_ERROR);
412412
UnlockCoins();
413-
keyHolderStorage.ReturnAll();
413+
WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
414414
nTimeLastSuccessfulStep = GetTime();
415415
strLastMessage = CoinJoin::GetMessageByID(ERR_SESSION);
416416

@@ -521,7 +521,7 @@ void CCoinJoinClientSession::ProcessPoolStateUpdate(CCoinJoinStatusUpdate psssup
521521
WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- rejected by Masternode: %s\n", __func__, strMessageTmp.translated);
522522
SetState(POOL_STATE_ERROR);
523523
UnlockCoins();
524-
keyHolderStorage.ReturnAll();
524+
WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
525525
nTimeLastSuccessfulStep = GetTime();
526526
strLastMessage = strMessageTmp;
527527
break;
@@ -688,7 +688,7 @@ void CCoinJoinClientSession::CompletedTransaction(PoolMessage nMessageID)
688688
keyHolderStorage.KeepAll();
689689
WalletCJLogPrint(m_wallet, "CompletedTransaction -- success\n");
690690
} else {
691-
keyHolderStorage.ReturnAll();
691+
WITH_LOCK(m_wallet.cs_wallet, keyHolderStorage.ReturnAll());
692692
WalletCJLogPrint(m_wallet, "CompletedTransaction -- error\n");
693693
}
694694
UnlockCoins();

0 commit comments

Comments
 (0)