Page MenuHomePhabricator
Differential D7213

[backport#15402] Prevent callback overruns in InvalidateBlock and RewindBlockIndex
ClosedPublic
Actions

Authored by majcosta on Aug 18 2020, 15:53.

Details

Reviewers
jasonbcox
deadalnix
Group Reviewers
Restricted Project
Commits
rABCc0c79f53e5c7: [backport#15402] Prevent callback overruns in InvalidateBlock and…
Summary

https://github.com/bitcoin/bitcoin/pull/15402/commits/9ce9c37004440d6a329874dbf66b51666d497dcb

This thing has been merged (D4803) and reverted (D4959) before. Seems to be working correctly now, with the lock to m_cs_chainstate in ActivateBestChain having a corresponding one in UnwindBlock.

Concludes backport of Core PR15402

Test Plan
cmake -GNinja -DCMAKE_BUILD_TYPE=Debug
ninja check check-functional

also

build without this patch, invalidate block #6 or similarly deep, see that bitcoind memory usage just keeps going up, even past 30% on my 16GB machine. build with this patch and do the same, memory usage stays steady at ~12% no matter how far you disconnect blocks.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

majcosta created this revision.Aug 18 2020, 15:53
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 18 2020, 15:53
majcosta requested review of this revision.Aug 18 2020, 15:53
teamcity edited the summary of this revision. (Show Details)Aug 18 2020, 15:53

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

majcosta mentioned this in D6940: [backport#15971] validation: Add compile-time checking for negative locking requirement in LimitValidationInterfaceQueue.Aug 18 2020, 16:22
majcosta added a child revision: D6940: [backport#15971] validation: Add compile-time checking for negative locking requirement in LimitValidationInterfaceQueue.Aug 18 2020, 16:23
Harbormaster completed remote builds in B12418: Diff 22947.Aug 18 2020, 16:27
jasonbcox requested changes to this revision.Aug 18 2020, 17:44
jasonbcox added a subscriber: jasonbcox.

Please run TSAN on this diff since the revert was tested that way. We should expect it to pass if it's really fixed.

This revision now requires changes to proceed.Aug 18 2020, 17:44
majcosta added a comment.Aug 18 2020, 18:54

@bot build-tsan

majcosta requested review of this revision.Aug 18 2020, 20:21

can you see the build result with this link?

https://build.bitcoinabc.org/viewType.html?buildTypeId=BitcoinABC_BitcoinAbcStaging&tab=buildTypeChangeLog&branch_BitcoinAbcDiffs=refs%2Ftags%2Fphabricator%2Fdiff%2F22947

deadalnix accepted this revision.Aug 19 2020, 15:45
jasonbcox accepted this revision.Aug 20 2020, 23:57
This revision is now accepted and ready to land.Aug 20 2020, 23:57
Closed by commit rABCc0c79f53e5c7: [backport#15402] Prevent callback overruns in InvalidateBlock and… (authored by Pieter Wuille <pieter.wuille@gmail.com>, committed by majcosta). · Explain WhyAug 21 2020, 15:06
This revision was automatically updated to reflect the committed changes.
majcosta added a commit: rABCc0c79f53e5c7: [backport#15402] Prevent callback overruns in InvalidateBlock and….
Fabien mentioned this in D13133: mempool: Add topological index, enforce consistency, updated reorg logic.Feb 15 2023, 16:49
Fabien mentioned this in rABCeec0e0c7b0f0: mempool: Add topological index, enforce consistency, updated reorg logic.Feb 21 2023, 10:32

Revision Contents
Changeset List

PathSize
src/
27 lines

Diff 23000

View Options

src/validation.cpp

Loading...