Prevent race conditions when getting list of accounts during scanning #5094
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
andiflabs
force-pushed
the
andrea/scanner-queue
branch
from
6120a33 to
518c427
Compare
Calling `Wallet.reset()` while a scanning is running can sometimes result in race conditions. In particular, `WalletScanner` may attempt to query account heads at the same time while accounts are being reset, resulting in errors. This commit fixes the problem by using database transactions (1) while `WalletScanner` read accounts and (2) while `Wallet.reset()` updates accounts. This commit also adds transactions in `Wallet.getEarliestHead()` and `Wallet.getLatestHead()`. These methods are not used during scanning, but they suffer from the same kind of problem. Note that this commit fixes specific race conditions related to getting the list of accounts to scan, but other race conditions that affect scanning may still exist.
andiflabs
force-pushed
the
andrea/wallet-head-tx
branch
from
ac11526 to
0390b5c
Compare
hughy
approved these changes
Jun 28, 2024
NullSoldier
approved these changes
Jul 1, 2024
|
one day we'll remove the need for this TX and just use a lock that you can take when you are modifying or using accounts. |
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.
Summary
Calling
Wallet.reset()while a scanning is running can sometimes result in race conditions. In particular,WalletScannermay attempt to query account heads at the same time while accounts are being reset, resulting in errors.This commit fixes the problem by using database transactions (1) while
WalletScannerread accounts and (2) whileWallet.reset()updates accounts.This commit also adds transactions in
Wallet.getEarliestHead()andWallet.getLatestHead(). These methods are not used during scanning, but they suffer from the same kind of problem.Note that this commit fixes specific race conditions related to getting the list of accounts to scan, but other race conditions that affect scanning may still exist.
Testing Plan
Documentation
N/A
Breaking Change
N/A