I don’t think this is critical to fix but if you wanted to remove the race condition, the following change should work:
0--- a/src/rpc/blockchain.cpp
1+++ b/src/rpc/blockchain.cpp
2@@ -74,6 +74,22 @@ static GlobalMutex cs_blockchange;
3 static std::condition_variable cond_blockchange;
4 static CUpdatedBlock latestblock GUARDED_BY(cs_blockchange);
5
6+std::tuple<std::unique_ptr<CCoinsViewCursor>, CCoinsStats, const CBlockIndex*>
7+PrepareUTXOSnapshot(
8+ Chainstate& chainstate,
9+ const std::function<void()>& interruption_point = {})
10+ EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
11+
12+UniValue WriteUTXOSnapshot(
13+ Chainstate& chainstate,
14+ CCoinsViewCursor* pcursor,
15+ CCoinsStats* maybe_stats,
16+ const CBlockIndex* tip,
17+ AutoFile& afile,
18+ const fs::path& path,
19+ const fs::path& temppath,
20+ const std::function<void()>& interruption_point = {});
21+
22 /* Calculate the difficulty for a given block index.
23 */
24 double GetDifficulty(const CBlockIndex& blockindex)
25@@ -2730,7 +2746,7 @@ static RPCHelpMan dumptxoutset()
26 target_index = ParseHashOrHeight(request.params[1], *node.chainman);
27 }
28
29- const auto tip{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};
30+ const CBlockIndex* tip{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};
31 const CBlockIndex* invalidate_index{nullptr};
32 std::unique_ptr<NetworkDisable> disable_network;
33
34@@ -2754,32 +2770,41 @@ static RPCHelpMan dumptxoutset()
35 // seems wrong in this temporary state. For example a normal new block
36 // would be classified as a block connecting an invalid block.
37 disable_network = std::make_unique<NetworkDisable>(connman);
38-
39- // Note: Unlocking cs_main before CreateUTXOSnapshot might be racy
40- // if the user interacts with any other *block RPCs.
41 invalidate_index = WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Next(target_index));
42 InvalidateBlock(*node.chainman, invalidate_index->GetBlockHash());
43- const CBlockIndex* new_tip_index{WITH_LOCK(::cs_main, return node.chainman->ActiveChain().Tip())};
44+ }
45
46+ Chainstate* chainstate;
47+ std::unique_ptr<CCoinsViewCursor> cursor;
48+ CCoinsStats stats;
49+ UniValue result;
50+ UniValue error;
51+ {
52+ LOCK(node.chainman->GetMutex());
53+ chainstate = &node.chainman->ActiveChainstate();
54 // In case there is any issue with a block being read from disk we need
55 // to stop here, otherwise the dump could still be created for the wrong
56 // height.
57 // The new tip could also not be the target block if we have a stale
58 // sister block of invalidate_index. This block (or a descendant) would
59 // be activated as the new tip and we would not get to new_tip_index.
60- if (new_tip_index != target_index) {
61- ReconsiderBlock(*node.chainman, invalidate_index->GetBlockHash());
62- throw JSONRPCError(RPC_MISC_ERROR, "Could not roll back to requested height, reverting to tip.");
63+ if (target_index != chainstate->m_chain.Tip()) {
64+ error = JSONRPCError(RPC_MISC_ERROR, "Could not roll back to requested height, reverting to tip.");
65+ } else {
66+ std::tie(cursor, stats, tip) = PrepareUTXOSnapshot(*chainstate, node.rpc_interruption_point);
67 }
68 }
69
70- UniValue result = CreateUTXOSnapshot(
71- node, node.chainman->ActiveChainstate(), afile, path, temppath);
72- fs::rename(temppath, path);
73-
74+ if (error.isNull()) {
75+ result = WriteUTXOSnapshot(*chainstate, cursor.get(), &stats, tip, afile, path, temppath, node.rpc_interruption_point);
76+ fs::rename(temppath, path);
77+ }
78 if (invalidate_index) {
79 ReconsiderBlock(*node.chainman, invalidate_index->GetBlockHash());
80 }
81+ if (!error.isNull()) {
82+ throw error;
83+ }
84
85 result.pushKV("path", path.utf8string());
86 return result;
87@@ -2787,12 +2812,10 @@ static RPCHelpMan dumptxoutset()
88 };
89 }
90
91-UniValue CreateUTXOSnapshot(
92- NodeContext& node,
93+std::tuple<std::unique_ptr<CCoinsViewCursor>, CCoinsStats, const CBlockIndex*>
94+PrepareUTXOSnapshot(
95 Chainstate& chainstate,
96- AutoFile& afile,
97- const fs::path& path,
98- const fs::path& temppath)
99+ const std::function<void()>& interruption_point)
100 {
101 std::unique_ptr<CCoinsViewCursor> pcursor;
102 std::optional<CCoinsStats> maybe_stats;
103@@ -2811,11 +2834,11 @@ UniValue CreateUTXOSnapshot(
104 // See discussion here:
105 // [#15606 (review)](/bitcoin-bitcoin/15606/#discussion_r274479369)
106 //
107- LOCK(::cs_main);
108+ AssertLockHeld(::cs_main);
109
110 chainstate.ForceFlushStateToDisk();
111
112- maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, node.rpc_interruption_point);
113+ maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, interruption_point);
114 if (!maybe_stats) {
115 throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set");
116 }
117@@ -2823,7 +2846,19 @@ UniValue CreateUTXOSnapshot(
118 pcursor = chainstate.CoinsDB().Cursor();
119 tip = CHECK_NONFATAL(chainstate.m_blockman.LookupBlockIndex(maybe_stats->hashBlock));
120 }
121+ return {std::move(pcursor), *CHECK_NONFATAL(maybe_stats), tip};
122+}
123
124+UniValue WriteUTXOSnapshot(
125+ Chainstate& chainstate,
126+ CCoinsViewCursor* pcursor,
127+ CCoinsStats* maybe_stats,
128+ const CBlockIndex* tip,
129+ AutoFile& afile,
130+ const fs::path& path,
131+ const fs::path& temppath,
132+ const std::function<void()>& interruption_point)
133+{
134 LOG_TIME_SECONDS(strprintf("writing UTXO snapshot at height %s (%s) to file %s (via %s)",
135 tip->nHeight, tip->GetBlockHash().ToString(),
136 fs::PathToString(path), fs::PathToString(temppath)));
137@@ -2859,7 +2894,7 @@ UniValue CreateUTXOSnapshot(
138 pcursor->GetKey(key);
139 last_hash = key.hash;
140 while (pcursor->Valid()) {
141- if (iter % 5000 == 0) node.rpc_interruption_point();
142+ if (iter % 5000 == 0) interruption_point();
143 ++iter;
144 if (pcursor->GetKey(key) && pcursor->GetValue(coin)) {
145 if (key.hash != last_hash) {
146@@ -2890,6 +2925,17 @@ UniValue CreateUTXOSnapshot(
147 return result;
148 }
149
150+UniValue CreateUTXOSnapshot(
151+ node::NodeContext& node,
152+ Chainstate& chainstate,
153+ AutoFile& afile,
154+ const fs::path& path,
155+ const fs::path& tmppath)
156+{
157+ auto [cursor, stats, tip]{WITH_LOCK(::cs_main, return PrepareUTXOSnapshot(chainstate, node.rpc_interruption_point))};
158+ return WriteUTXOSnapshot(chainstate, cursor.get(), &stats, tip, afile, path, tmppath, node.rpc_interruption_point);
159+}
160+
161 static RPCHelpMan loadtxoutset()
162 {
163 return RPCHelpMan{