coins: prevent DB resize from invalidating cursors #35744

pull l0rinc wants to merge 1 commits into bitcoin:master from l0rinc:l0rinc/coins-cursor-resize-lifetime changing 3 files +50 −11

l0rinc commented at 10:25 PM on July 17, 2026: contributor
Problem: gettxoutsetinfo, scantxoutset, and dumptxoutset can retain LevelDB iterators while AssumeUTXO cache rebalancing replaces m_db, causing LevelDB to abort.

Originally reported in #35465 (review).

Fix: Make ResizeCache() wait for live cursors and background compaction before replacing m_db; cursor iteration and compaction remain concurrent. A resize can wait for the duration of a long-running UTXO scan.

<details> <summary>LevelDB assertion failure without the fix</summary>
```
Assertion failed: (dummy_versions_.next_ == &dummy_versions_), function ~VersionSet, file version_set.cc, line 753.
```
</details>

coins: block DB resize during cursor iteration

`gettxoutsetinfo`, `scantxoutset`, and `dumptxoutset` retain LevelDB iterators after releasing `cs_main`. AssumeUTXO cache rebalancing can concurrently call `ResizeCache()`, destroying `m_db` while those iterators remain alive.
Originally reported in https://github.com/bitcoin/bitcoin/pull/35465#discussion_r3428902672.

Count live cursors and make `ResizeCache()` wait for them and any background compaction before replacing `m_db`. Cursor iteration and compaction remain concurrent.

Co-authored-by: Pieter Wuille <pieter@wuille.net>
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

d3484251b3

DrahtBot added the label UTXO Db and Indexes on Jul 17, 2026
DrahtBot commented at 10:26 PM on July 17, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35744.



Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
in src/txdb.h:45 in d3484251b3
```
  38 | @@ -38,9 +39,12 @@ class CCoinsViewDB final : public CCoinsView
  39 |  protected:
  40 |      DBParams m_db_params;
  41 |      CoinsViewOptions m_options;
  42 | -    //! Prevents CompactFull() from using m_db while ResizeCache() replaces it.
  43 | -    Mutex m_db_mutex;
  44 | +    //! Prevents cursor creation while ResizeCache() replaces m_db.
  45 | +    mutable Mutex m_db_mutex;
  46 | +    //! Live cursors blocking ResizeCache() from replacing m_db.
  47 | +    mutable std::atomic_int m_cursor_count{0};
```
andrewtoth commented at 2:31 AM on July 18, 2026:

Thanks for tackling this.

I'm not sure about this approach with an atomic counter though. Would it make sense to instead have a shared mutex that would allow multiple consumers to take a read lock along with the cursor, which are then released in the destructor? Then only ResizeCache takes an exclusive lock? Just brainstorming here.

l0rinc commented at 4:45 AM on July 18, 2026:

Thanks, I already investigated this, but the warnings and LLM reviews scared me off because it seems to force cursors to be destroyed on the thread that created them: unlock_shared() requires the calling thread to hold the shared lock. Current callers seem to satisfy that, though, so maybe it is fine. I also found that std::shared_mutex may lose some of Core's lock checking: this Godbolt recreates it, which still warns inside a std::unique_lock<std::shared_mutex>, and my local DEBUG_LOCKORDER probe did not see the lock. What do you think?

andrewtoth commented at 9:41 PM on July 18, 2026:

it seems to force cursors to be destroyed on the thread that created them

Iterators alone are not thread-safe though, so we should not be passing them to other threads. I think in this case it should be fine.

std::shared_mutex may lose some of Core's lock checking

Do we need this here? We do use a shared_mutex already in SignatureCache without any of the lock checking machinery.

Contributors

Labels

Linked (view graph)

#35465 coins: compact chainstate regularly

coins: prevent DB resize from invalidating cursors #35744

Code Coverage & Benchmarks

Reviews

Conflicts