From 3d1150a1a853ea57059ca1ad41c558d5343a31bd Mon Sep 17 00:00:00 2001 From: Calvin Kim Date: Tue, 27 Feb 2024 14:32:42 +0900 Subject: [PATCH] blockchain: always relock chainLock for subscription callbacks For various b.sendNotifcation() callbacks, if a runtime panic happens, we don't get any useful debugging information since the error that happens first is the "unlock of unlocked mutex" error. This is because we temporarily unlock the chainLock for callbacks and then relock them. However, since the relocking code is executed after the completion of the callback, if an error happens during that callback, we never relock the chainLock. Switching to an anonymous function and having the unlock code as a defer will ensure that the lock always relocks. --- blockchain/accept.go | 8 +++++--- blockchain/chain.go | 16 ++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/blockchain/accept.go b/blockchain/accept.go index 935963148f..4adc2f6127 100644 --- a/blockchain/accept.go +++ b/blockchain/accept.go @@ -84,9 +84,11 @@ func (b *BlockChain) maybeAcceptBlock(block *btcutil.Block, flags BehaviorFlags) // Notify the caller that the new block was accepted into the block // chain. The caller would typically want to react by relaying the // inventory to other peers. - b.chainLock.Unlock() - b.sendNotification(NTBlockAccepted, block) - b.chainLock.Lock() + func() { + b.chainLock.Unlock() + defer b.chainLock.Lock() + b.sendNotification(NTBlockAccepted, block) + }() return isMainChain, nil } diff --git a/blockchain/chain.go b/blockchain/chain.go index 60420022ac..416ed92931 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -730,9 +730,11 @@ func (b *BlockChain) connectBlock(node *blockNode, block *btcutil.Block, // Notify the caller that the block was connected to the main chain. // The caller would typically want to react with actions such as // updating wallets. - b.chainLock.Unlock() - b.sendNotification(NTBlockConnected, block) - b.chainLock.Lock() + func() { + b.chainLock.Unlock() + defer b.chainLock.Lock() + b.sendNotification(NTBlockConnected, block) + }() // Since we may have changed the UTXO cache, we make sure it didn't exceed its // maximum size. If we're pruned and have flushed already, this will be a no-op. @@ -853,9 +855,11 @@ func (b *BlockChain) disconnectBlock(node *blockNode, block *btcutil.Block, view // Notify the caller that the block was disconnected from the main // chain. The caller would typically want to react with actions such as // updating wallets. - b.chainLock.Unlock() - b.sendNotification(NTBlockDisconnected, block) - b.chainLock.Lock() + func() { + b.chainLock.Unlock() + defer b.chainLock.Lock() + b.sendNotification(NTBlockDisconnected, block) + }() return nil }