backport: bitcoin#18742, #28150 #5726
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 tasks
knst
approved these changes
Nov 24, 2023
| @@ -955,7 +955,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) | |||
| return result; | |||
| } | |||
|
|
|||
| class submitblock_StateCatcher : public CValidationInterface | |||
| class submitblock_StateCatcher final : public CValidationInterface | |||
There was a problem hiding this comment.
nit: Should this one be updated also (missing final)?
src/dsnotificationinterface.h:class CDSNotificationInterface : public CValidationInterface
PastaPastaPasta
approved these changes
Nov 26, 2023
…ninterface 7777f2a miner: Avoid stack-use-after-return in validationinterface (MarcoFalke) fa5ceb2 test: Remove UninterruptibleSleep from test and replace it by SyncWithValidationInterfaceQueue (MarcoFalke) fa770ce validationinterface: Rework documentation, Rename pwalletIn to callbacks (MarcoFalke) fab6d06 test: Add unregister_validation_interface_race test (MarcoFalke) Pull request description: When a validationinterface has itself unregistered in one thread, but is about to get executed in another thread [1], there is a race: * The validationinterface destructing itself * The validationinterface getting dereferenced for execution [1] https://github.com/bitcoin/bitcoin/blob/64139803f1225dab26197a20314109d37fa87d5f/src/validationinterface.cpp#L82-L83 This happens in the miner. More generally it happens everywhere where at least one thread is generating notifications and another one is unregistering a validationinterface. This issue has been fixed in commit ab31b9d, but the fix has not been applied to the miner. Example where this happened in practice: https://travis-ci.org/github/bitcoin/bitcoin/jobs/675322230#L4414 ACKs for top commit: promag: Code review ACK 7777f2a. laanwj: Code review ACK 7777f2a Tree-SHA512: 8087119243c71ba18a823a63515f3730d127162625d8729024278b447af29e2ff206f4840ee3d90bf84f93a2c5ab73b76c7e7044c83aa93b5b51047a166ec3d3
…nts in validationinterface_tests faca9a3 test: Avoid intermittent issues due to async events in validationinterface_tests (MarcoFalke) Pull request description: Currently the tests have many issues: * They setup the genesis block, even though it is not needed * They queue an async `UpdatedBlockTip` even, which causes intermittent issues: bitcoin#28146 (comment) Fix all issues by trimming down the setup to just `ChainTestingSetup`. ACKs for top commit: Crypt-iQ: tACK faca9a3 Tree-SHA512: 4449040330f89bbaf5ce5b2052417c160b451c373987fdf1069596c07834ed81f0aea1506d53c7d2cd21062b27332d30679285dae194b272fd0cb9ce5ded32cf
PastaPastaPasta
force-pushed
the
bp_18742_28150
branch
from
5fdb05c to
2d631df
Compare
ogabrielides
pushed a commit
to ogabrielides/dash
that referenced
this pull request
Dec 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Should hopefully fix random issues like https://gitlab.com/dashpay/dash/-/jobs/5586877812 observed in #5724
What was done?
How Has This Been Tested?
run tests
Breaking Changes
n/a
Checklist: