Fix locking in GetTransaction. #6688

pull morcos wants to merge 1 commits into bitcoin:master from morcos:lock_GetTransaction changing 1 files +33 −35

morcos commented at 9:55 PM on September 17, 2015: member

GetTransaction needs to lock cs_main until ReadBlockFromDisk completes because the data inside CBlockIndex's can change since pruning. This lock was held by all calls to GetTransaction except rest_tx.
sipa commented at 11:07 PM on September 17, 2015: member

Concept ACk
dcousens commented at 11:26 PM on September 17, 2015: contributor

@morcos in this case, you can just remove that scoping block.

utACK

Fix locking in GetTransaction.

GetTransaction needs to lock cs_main until ReadBlockFromDisk completes, the data inside CBlockIndex's can change since pruning.  This lock was held by all calls to GetTransaction except rest_tx.

131c23d027

laanwj commented at 12:59 AM on September 18, 2015: member

This is unfortunate. Can we do reading the block from disk without lock, at least? E.g. the i/o itself. cs_main is extremely contended, it's not nice to hold it during longer operations.

Though utACK anyway...
laanwj added the label Bug on Sep 18, 2015
morcos force-pushed on Sep 18, 2015
morcos commented at 3:10 AM on September 18, 2015: member

@dcousens oops, thanks.. also removed another unnecessary scoping block.
dcousens commented at 8:43 AM on September 19, 2015: contributor

utACK, agreed with @laanwj, but that is something we should address in another PR IMHO.
dcousens cross-referenced this on Sep 20, 2015 from issue use CBlockIndex* insted of uint256 for UpdatedBlockTip signal by jonasschnelli
laanwj merged this on Sep 23, 2015
laanwj closed this on Sep 23, 2015
laanwj referenced this in commit 4939eabb75 on Sep 23, 2015
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#6680 use CBlockIndex* insted of uint256 for UpdatedBlockTip signal