Fix missing wallet lock in CWallet::SyncTransaction(..) #3849

pull cozz wants to merge 1 commits into bitcoin:master from cozz:cozz2 changing 1 files +3 −4

cozz commented at 10:46 PM on March 11, 2014: contributor
Looks like we are currently accessing mapWallet without a lock held in CWallet::SyncTransaction(..). I tested this with AssertLockHeld(cs_wallet) and indeed, no lock.
- moved the lock from AddToWalletIfInvolvingMe(..) to SyncTransaction(..)
- using return value of AddToWalletIfInvolvingMe(..) to determine whether the tx is in the wallet
Fix missing wallet lock in CWallet::SyncTransaction(..) 53d56881a8
laanwj commented at 2:32 PM on March 13, 2014: member

I see the other use of AddToWalletIfInvolvingMe already takes the lock. Ok. Maybe document that the function needs to be called with the lock held.

Relying on the return value from CWallet::AddToWalletIfInvolvingMe should be safe as fUpdate is true. It will always return true unless the transaction is not in the wallet and not involving me, or a write problem happened in wtx.WriteToDisk().

ACK
cozz commented at 3:07 PM on March 13, 2014: contributor

@laanwj yes, exactly what I was thinking.
gavinandresen commented at 3:43 PM on March 13, 2014: contributor

ACK
gavinandresen referenced this in commit 1e13f57f56 on Mar 13, 2014
gavinandresen merged this on Mar 13, 2014
gavinandresen closed this on Mar 13, 2014
DrahtBot locked this on Sep 8, 2021

Contributors