[net processing] Guard m_continuation_block with m_block_inv_mutex

jnewbery · jnewbery · commit 3002b4af2b4f · 2020-12-20T10:06:14.000Z
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -1615,15 +1615,18 @@ void static ProcessGetBlockData(CNode& pfrom, Peer& peer, const CChainParams& ch
             }
         }
 
-        // Trigger the peer node to send a getblocks request for the next batch of inventory
-        if (inv.hash == peer.m_continuation_block) {
-            // Send immediately. This must send even if redundant,
-            // and we want it right after the last block so they don't
-            // wait for other stuff first.
-            std::vector<CInv> vInv;
-            vInv.push_back(CInv(MSG_BLOCK, ::ChainActive().Tip()->GetBlockHash()));
-            connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv));
-            peer.m_continuation_block.SetNull();
+        {
+            LOCK(peer.m_block_inv_mutex);
+            // Trigger the peer node to send a getblocks request for the next batch of inventory
+            if (inv.hash == peer.m_continuation_block) {
+                // Send immediately. This must send even if redundant,
+                // and we want it right after the last block so they don't
+                // wait for other stuff first.
+                std::vector<CInv> vInv;
+                vInv.push_back(CInv(MSG_BLOCK, ::ChainActive().Tip()->GetBlockHash()));
+                connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv));
+                peer.m_continuation_block.SetNull();
+            }
         }
     }
 }
@@ -2799,12 +2802,11 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
                 break;
             }
             WITH_LOCK(peer->m_block_inv_mutex, peer->m_blocks_for_inv_relay.push_back(pindex->GetBlockHash()));
-            if (--nLimit <= 0)
-            {
+            if (--nLimit <= 0) {
                 // When this block is requested, we'll send an inv that'll
                 // trigger the peer to getblocks the next batch of inventory.
                 LogPrint(BCLog::NET, "  getblocks stopping at limit %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());
-                peer->m_continuation_block = pindex->GetBlockHash();
+                WITH_LOCK(peer->m_block_inv_mutex, {peer->m_continuation_block = pindex->GetBlockHash();});
                 break;
             }
         }
diff --git a/src/net_processing.h b/src/net_processing.h
@@ -73,14 +73,14 @@ struct Peer {
      * message. If we can't announce via a `headers` message, we'll fall back to
      * announcing via `inv`. */
     std::vector<uint256> m_blocks_for_headers_relay GUARDED_BY(m_block_inv_mutex);
-
-    /** This peer's reported block height when we connected */
-    std::atomic<int> m_starting_height{-1};
     /** The final block hash that we sent in an `inv` message to this peer.
      * When the peer requests this block, we send an `inv` message to trigger
      * the peer to request the next sequence of block hashes.
      * Most peers use headers-first syncing, which doesn't use this mechanism */
-    uint256 m_continuation_block{};
+    uint256 m_continuation_block GUARDED_BY(m_block_inv_mutex) {};
+
+    /** This peer's reported block height when we connected */
+    std::atomic<int> m_starting_height{-1};
 
     /** Set of txids to reconsider once their parent transactions have been accepted **/
     std::set<uint256> m_orphan_work_set GUARDED_BY(g_cs_orphans);

Original file line number	Diff line number	Diff line change
`@@ -1615,15 +1615,18 @@ void static ProcessGetBlockData(CNode& pfrom, Peer& peer, const CChainParams& ch`
`1615`	`1615`	`}`
`1616`	`1616`	`}`
`1617`	`1617`
`1618`		`- // Trigger the peer node to send a getblocks request for the next batch of inventory`
`1619`		`- if (inv.hash == peer.m_continuation_block) {`
`1620`		`- // Send immediately. This must send even if redundant,`
`1621`		`- // and we want it right after the last block so they don't`
`1622`		`- // wait for other stuff first.`
`1623`		`- std::vector<CInv> vInv;`
`1624`		`- vInv.push_back(CInv(MSG_BLOCK, ::ChainActive().Tip()->GetBlockHash()));`
`1625`		`- connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv));`
`1626`		`- peer.m_continuation_block.SetNull();`
	`1618`	`+ {`
	`1619`	`+ LOCK(peer.m_block_inv_mutex);`
	`1620`	`+ // Trigger the peer node to send a getblocks request for the next batch of inventory`
	`1621`	`+ if (inv.hash == peer.m_continuation_block) {`
	`1622`	`+ // Send immediately. This must send even if redundant,`
	`1623`	`+ // and we want it right after the last block so they don't`
	`1624`	`+ // wait for other stuff first.`
	`1625`	`+ std::vector<CInv> vInv;`
	`1626`	`+ vInv.push_back(CInv(MSG_BLOCK, ::ChainActive().Tip()->GetBlockHash()));`
	`1627`	`+ connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv));`
	`1628`	`+ peer.m_continuation_block.SetNull();`
	`1629`	`+ }`
`1627`	`1630`	`}`
`1628`	`1631`	`}`
`1629`	`1632`	`}`
`@@ -2799,12 +2802,11 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat`
`2799`	`2802`	`break;`
`2800`	`2803`	`}`
`2801`	`2804`	`WITH_LOCK(peer->m_block_inv_mutex, peer->m_blocks_for_inv_relay.push_back(pindex->GetBlockHash()));`
`2802`		`- if (--nLimit <= 0)`
`2803`		`- {`
	`2805`	`+ if (--nLimit <= 0) {`
`2804`	`2806`	`// When this block is requested, we'll send an inv that'll`
`2805`	`2807`	`// trigger the peer to getblocks the next batch of inventory.`
`2806`	`2808`	`LogPrint(BCLog::NET, " getblocks stopping at limit %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());`
`2807`		`- peer->m_continuation_block = pindex->GetBlockHash();`
	`2809`	`+ WITH_LOCK(peer->m_block_inv_mutex, {peer->m_continuation_block = pindex->GetBlockHash();});`
`2808`	`2810`	`break;`
`2809`	`2811`	`}`
`2810`	`2812`	`}`