mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-09-14 07:01:16 +02:00
9e62e4b1f3test: slow down rescaning process (Pol Espinasa)336f5a738bwallet: reserve walletrescan before checking wallet is at the tip (Pol Espinasa) Pull request description: `ImportDescriptors` rpc has a race condition where two imports running in parallel can both succeed or fail one of them. The race happens when there are two threads A and B trying to importdescriptors at the same time. 1. Thread A calls `BlockUntilSyncedToCurrentChain()` (holding `cs_wallet` fast, no contention) and then `reserve()`, acquiring the `WalletRescanReserver`. It proceeds to `ProcessDescriptorImport()`, which holds `cs_wallet` for an extended time (specially on slow machines) while importing descriptors. 2. B reaches `BlockUntilSyncedToCurrentChain()`, which internally does `WITH_LOCK(cs_wallet, ...)`. Since A holds `cs_wallet`, B blocks here for the entire duration of Thread A's descriptors import. 3. Then A finishes importing, releases `cs_wallet`, rescans (fast in regtest), and sets `fScanningWallet = false`. 4. B can now continue in `BlockUntilSyncedToCurrentChain()` acquiring `cs_wallet`, and then calls `reserve()` which succeeds because `fScanningWallet` is already `false`. Both imports succeed. I don't think the behavior is problematic at all from a usability PoV, but it can be a bad UX if some imports fails and other's no. It also makes testing difficult as race conditions are not easy to test. This PR fixes it by calling `reserver.reserve()` before `cs_wallet` is locked, so multiple threads will be aware of currently imports before being stuck at any point. So only one `importdescriptor` call can be done at the same time. I think this should fix https://github.com/bitcoin/bitcoin/issues/35544#issuecomment-4763488259 ACKs for top commit: achow101: ACK9e62e4b1f3nebula-21: ACK9e62e4b1f3w0xlt: lgtm reACK9e62e4b1f3Tree-SHA512: be0027e1a7b77252ed9fb514c3b3311d6905903d4b0bfc1021a1e1c2bb06872ef599647ff8a4536929240717ae9db62fb75374f629cd3046c7192e2b8b4d7344