Merge #19178: Make mininode_lock non-reentrant

6206838 [tests] Make mininode_lock non-reentrant (John Newbery)
c67c1f2 [tests] Don't call super twice in P2PTxInvStore.on_inv() (John Newbery)
9d80762 [tests] Don't acquire mininode_lock twice in wait_for_broadcast() (John Newbery)
edae607 [tests] Only acquire lock once in p2p_compactblocks.py (John Newbery)

Pull request description:

  There's no need for mininode_lock to be reentrant.
  Use a simpler non-recursive lock.

ACKs for top commit:
  MarcoFalke:
    ACK 6206838 😃
  jonatack:
    ACK 6206838

Tree-SHA512: dcbc19e6c986970051705789be0ff7bec70c69cf76d5b468c2ba4cb732883ad512b1de5c3206c2eca41fa3f1c4806999df4cabbf67fc3c463bb817458e59a19c

Author: MarcoFalke

test/functional/p2p_compactblocks.py

@@ -305,10 +305,9 @@ def test_compactblock_construction(self, test_node, use_witness_address=True):
         self.check_compactblock_construction_from_block(version, header_and_shortids, block_hash, block)
 
         # Now fetch the compact block using a normal non-announce getdata
-        with mininode_lock:
-            test_node.clear_block_announcement()
-            inv = CInv(MSG_CMPCT_BLOCK, block_hash)
-            test_node.send_message(msg_getdata([inv]))
+        test_node.clear_block_announcement()
+        inv = CInv(MSG_CMPCT_BLOCK, block_hash)
+        test_node.send_message(msg_getdata([inv]))
 
         wait_until(test_node.received_block_announcement, timeout=30, lock=mininode_lock)
 

test/functional/test_framework/mininode.py

@@ -492,7 +492,7 @@ def test_function():
 # P2PConnection acquires this lock whenever delivering a message to a P2PInterface.
 # This lock should be acquired in the thread running the test logic to synchronize
 # access to any data shared with the P2PInterface or P2PConnection.
-mininode_lock = threading.RLock()
+mininode_lock = threading.Lock()
 
 
 class NetworkThread(threading.Thread):
@@ -658,8 +658,6 @@ def on_inv(self, message):
                 # save txid
                 self.tx_invs_received[i.hash] += 1
 
-        super().on_inv(message)
-
     def get_invs(self):
         with mininode_lock:
             return list(self.tx_invs_received.keys())
@@ -669,6 +667,6 @@ def wait_for_broadcast(self, txns, timeout=60):
         The mempool should mark unbroadcast=False for these transactions.
         """
         # Wait until invs have been received (and getdatas sent) for each txid.
-        self.wait_until(lambda: set(self.get_invs()) == set([int(tx, 16) for tx in txns]), timeout)
+        self.wait_until(lambda: set(self.tx_invs_received.keys()) == set([int(tx, 16) for tx in txns]), timeout)
         # Flush messages and wait for the getdatas to be processed
         self.sync_with_ping()