Merge bitcoin#11849: [tests] Assert that only one NetworkThread exists

5c8ff26 [tests] Add NetworkThread assertions (John Newbery)
34e08b3 [tests] Fix network threading in functional tests (John Newbery)
74e64f2 [tests] Use network_thread_start() in tests. (John Newbery)
5fc6e71 [tests] Add network_thread_ utility functions. (John Newbery)

Pull request description:

  Add assert that only one NetworkThread exists at any time in functional tests, and fix cases where that wasn't true.

  fixes bitcoin#11776

Tree-SHA512: fe5d1c59005f94bf66e11bb23ccf274b1cd9913741b56ea11dbcd21db4cc0b53b4413c0c4c16dbcd6ac611adad5e5cc2baaa39720598ce7b6393889945d06298

test/functional/README.md

@@ -68,7 +68,7 @@ a callback class that derives from `NodeConnCB` and pass that to the
 `NodeConn` object, your code will receive the appropriate callbacks when
 events of interest arrive.
 
-- Call `NetworkThread.start()` after all `NodeConn` objects are created to
+- Call `network_thread_start()` after all `NodeConn` objects are created to
 start the networking thread.  (Continue with the test logic in your existing
 thread.)
 

test/functional/assumevalid.py

@@ -38,7 +38,8 @@
                                      CTransaction,
                                      CTxIn,
                                      CTxOut,
-                                     NetworkThread,
+                                     network_thread_join,
+                                     network_thread_start,
                                      NodeConnCB,
                                      msg_block,
                                      msg_headers)
@@ -103,7 +104,7 @@ def run_test(self):
         # Connect to node0
         p2p0 = self.nodes[0].add_p2p_connection(BaseNode())
 
-        NetworkThread().start()  # Start up network handling in another thread
+        network_thread_start()
         self.nodes[0].p2p.wait_for_verack()
 
         # Build the blockchain
@@ -164,13 +165,22 @@ def run_test(self):
             self.block_time += 1
             height += 1
 
+        # We're adding new connections so terminate the network thread
+        self.nodes[0].disconnect_p2ps()
+        network_thread_join()
+
         # Start node1 and node2 with assumevalid so they accept a block with a bad signature.
         self.start_node(1, extra_args=["-assumevalid=" + hex(block102.sha256)])
-        p2p1 = self.nodes[1].add_p2p_connection(BaseNode())
-        p2p1.wait_for_verack()
-
         self.start_node(2, extra_args=["-assumevalid=" + hex(block102.sha256)])
+
+        p2p0 = self.nodes[0].add_p2p_connection(BaseNode())
+        p2p1 = self.nodes[1].add_p2p_connection(BaseNode())
         p2p2 = self.nodes[2].add_p2p_connection(BaseNode())
+
+        network_thread_start()
+
+        p2p0.wait_for_verack()
+        p2p1.wait_for_verack()
         p2p2.wait_for_verack()
 
         # Make sure nodes actually accept the many headers

test/functional/bip65-cltv-p2p.py

@@ -68,7 +68,7 @@ def set_test_params(self):
     def run_test(self):
         self.nodes[0].add_p2p_connection(NodeConnCB())
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
 
         # wait_for_verack ensures that the P2P connection is fully up.
         self.nodes[0].p2p.wait_for_verack()

test/functional/bip68-112-113-p2p.py

@@ -45,7 +45,7 @@
 
 from test_framework.test_framework import (ComparisonTestFramework, GENESISTIME)
 from test_framework.util import *
-from test_framework.mininode import ToHex, CTransaction, NetworkThread
+from test_framework.mininode import ToHex, CTransaction, network_thread_start
 from test_framework.blocktools import create_coinbase, create_block
 from test_framework.comptool import TestInstance, TestManager
 from test_framework.script import *
@@ -105,7 +105,7 @@ def setup_network(self):
     def run_test(self):
         test = TestManager(self, self.options.tmpdir)
         test.add_all_connections(self.nodes)
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         test.run()
 
     def send_generic_input_tx(self, node, coinbases):

test/functional/bip9-softforks.py

@@ -22,7 +22,7 @@
 
 from test_framework.test_framework import ComparisonTestFramework
 from test_framework.util import *
-from test_framework.mininode import CTransaction, NetworkThread
+from test_framework.mininode import CTransaction, network_thread_start
 from test_framework.blocktools import create_coinbase, create_block
 from test_framework.comptool import TestInstance, TestManager
 from test_framework.script import CScript, OP_1NEGATE, OP_CHECKSEQUENCEVERIFY, OP_DROP
@@ -36,7 +36,7 @@ def set_test_params(self):
     def run_test(self):
         self.test = TestManager(self, self.options.tmpdir)
         self.test.add_all_connections(self.nodes)
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         self.test.run()
 
     def create_transaction(self, node, coinbase, to_address, amount):
@@ -244,7 +244,7 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu
         self.setup_chain()
         self.setup_network()
         self.test.add_all_connections(self.nodes)
-        NetworkThread().start()
+        network_thread_start()
         self.test.test_nodes[0].wait_for_verack()
 
     def get_tests(self):

test/functional/bipdersig-p2p.py

@@ -57,7 +57,7 @@ def set_test_params(self):
     def run_test(self):
         self.nodes[0].add_p2p_connection(NodeConnCB())
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
 
         # wait_for_verack ensures that the P2P connection is fully up.
         self.nodes[0].p2p.wait_for_verack()

test/functional/example_test.py

@@ -17,11 +17,12 @@
 from test_framework.blocktools import (create_block, create_coinbase)
 from test_framework.mininode import (
     CInv,
-    NetworkThread,
     NodeConnCB,
     mininode_lock,
     msg_block,
     msg_getdata,
+    network_thread_join,
+    network_thread_start,
 )
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -131,12 +132,12 @@ def custom_method(self):
     def run_test(self):
         """Main test logic"""
 
-        # Create a P2P connection to one of the nodes
+        # Create P2P connections to two of the nodes
         self.nodes[0].add_p2p_connection(BaseNode())
 
         # Start up network handling in another thread. This needs to be called
         # after the P2P connections have been created.
-        NetworkThread().start()
+        network_thread_start()
         # wait_for_verack ensures that the P2P connection is fully up.
         self.nodes[0].p2p.wait_for_verack()
 
@@ -188,7 +189,14 @@ def run_test(self):
         connect_nodes(self.nodes[1], 2)
 
         self.log.info("Add P2P connection to node2")
+        # We can't add additional P2P connections once the network thread has started. Disconnect the connection
+        # to node0, wait for the network thread to terminate, then connect to node2. This is specific to
+        # the current implementation of the network thread and may be improved in future.
+        self.nodes[0].disconnect_p2ps()
+        network_thread_join()
+
         self.nodes[2].add_p2p_connection(BaseNode())
+        network_thread_start()
         self.nodes[2].p2p.wait_for_verack()
 
         self.log.info("Wait for node2 reach current tip. Test that it has propogated all the blocks to us")

test/functional/invalidblockrequest.py

@@ -15,6 +15,7 @@
 from test_framework.util import *
 from test_framework.comptool import TestManager, TestInstance, RejectResult
 from test_framework.blocktools import *
+from test_framework.mininode import network_thread_start
 import copy
 
 # Use the ComparisonTestFramework with 1 node: only use --testbinary.
@@ -31,7 +32,7 @@ def run_test(self):
         test.add_all_connections(self.nodes)
         self.tip = None
         self.block_time = None
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         sync_masternodes(self.nodes, True)
         test.run()
 

test/functional/invalidtxrequest.py

@@ -26,7 +26,7 @@ def run_test(self):
         test.add_all_connections(self.nodes)
         self.tip = None
         self.block_time = None
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         test.run()
 
     def get_tests(self):

test/functional/maxuploadtarget.py

@@ -62,7 +62,7 @@ def run_test(self):
         for _ in range(3):
             p2p_conns.append(self.nodes[0].add_p2p_connection(TestNode()))
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         for p2pc in p2p_conns:
             p2pc.wait_for_verack()
 
@@ -154,7 +154,7 @@ def run_test(self):
         # Reconnect to self.nodes[0]
         self.nodes[0].add_p2p_connection(TestNode())
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         self.nodes[0].p2p.wait_for_verack()
 
         #retrieve 20 blocks which should be enough to break the 1MB limit

test/functional/nulldummy.py

@@ -15,7 +15,7 @@
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
-from test_framework.mininode import CTransaction, NetworkThread
+from test_framework.mininode import CTransaction, network_thread_start
 from test_framework.blocktools import create_coinbase, create_block
 from test_framework.script import CScript
 from io import BytesIO
@@ -45,7 +45,7 @@ def run_test(self):
         self.address = self.nodes[0].getnewaddress()
         self.ms_address = self.nodes[0].addmultisigaddress(1,[self.address])
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         self.coinbase_blocks = self.nodes[0].generate(2) # Block 2
         coinbase_txid = []
         for i in self.coinbase_blocks:

test/functional/p2p-acceptblock.py

@@ -83,7 +83,7 @@ def run_test(self):
         # min_work_node connects to node1
         min_work_node = self.nodes[1].add_p2p_connection(NodeConnCB())
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
 
         # Test logic begins here
         test_node.wait_for_verack()
@@ -207,9 +207,13 @@ def run_test(self):
         # disconnect/reconnect first
 
         self.nodes[0].disconnect_p2ps()
-        test_node = self.nodes[0].add_p2p_connection(NodeConnCB())
+        self.nodes[1].disconnect_p2ps()
+        network_thread_join()
 
+        test_node = self.nodes[0].add_p2p_connection(NodeConnCB())
+        network_thread_start()
         test_node.wait_for_verack()
+
         test_node.send_message(msg_block(block_h1f))
 
         test_node.sync_with_ping()
@@ -294,7 +298,7 @@ def run_test(self):
             self.nodes[0].disconnect_p2ps()
             test_node = self.nodes[0].add_p2p_connection(NodeConnCB())
 
-            NetworkThread().start() # Start up network handling in another thread
+            network_thread_start()
             test_node.wait_for_verack()
 
         # We should have failed reorg and switched back to 290 (but have block 291)

test/functional/p2p-compactblocks.py

@@ -716,7 +716,7 @@ def run_test(self):
         self.second_node = self.nodes[1].add_p2p_connection(TestNode(), services=NODE_NETWORK)
         self.old_node = self.nodes[1].add_p2p_connection(TestNode(), services=NODE_NETWORK)
 
-        NetworkThread().start()  # Start up network handling in another thread
+        network_thread_start()
 
         self.test_node.wait_for_verack()
 

test/functional/p2p-fingerprint.py

@@ -14,12 +14,12 @@
 from test_framework.blocktools import (create_block, create_coinbase)
 from test_framework.mininode import (
     CInv,
-    NetworkThread,
     NodeConnCB,
     msg_headers,
     msg_block,
     msg_getdata,
     msg_getheaders,
+    network_thread_start,
     wait_until,
 )
 from test_framework.test_framework import BitcoinTestFramework
@@ -79,7 +79,7 @@ def last_header_equals(self, expected_hash, node):
     def run_test(self):
         node0 = self.nodes[0].add_p2p_connection(NodeConnCB())
 
-        NetworkThread().start()
+        network_thread_start()
         node0.wait_for_verack()
 
         # Set node time to 60 days ago

test/functional/p2p-fullblocktest.py

@@ -17,6 +17,7 @@
 from test_framework.blocktools import *
 from test_framework.key import CECKey
 from test_framework.script import *
+from test_framework.mininode import network_thread_start
 import struct
 
 class PreviousSpendableOutput(object):
@@ -69,7 +70,7 @@ def add_options(self, parser):
     def run_test(self):
         self.test = TestManager(self, self.options.tmpdir)
         self.test.add_all_connections(self.nodes)
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
         sync_masternodes(self.nodes, True)
         self.test.run()
 

test/functional/p2p-leaktests.py

@@ -98,7 +98,7 @@ def run_test(self):
         no_version_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVersionIdle(), send_version=False)
         no_verack_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVerackIdle())
 
-        NetworkThread().start()  # Start up network handling in another thread
+        network_thread_start()
 
         wait_until(lambda: no_version_bannode.ever_connected, timeout=10, lock=mininode_lock)
         wait_until(lambda: no_version_idlenode.ever_connected, timeout=10, lock=mininode_lock)

test/functional/p2p-mempool.py

@@ -21,7 +21,7 @@ def set_test_params(self):
     def run_test(self):
         # Add a p2p connection
         self.nodes[0].add_p2p_connection(NodeConnCB())
-        NetworkThread().start()
+        network_thread_start()
         self.nodes[0].p2p.wait_for_verack()
 
         #request mempool

test/functional/p2p-timeouts.py

@@ -43,7 +43,7 @@ def run_test(self):
         no_version_node = self.nodes[0].add_p2p_connection(TestNode(), send_version=False)
         no_send_node = self.nodes[0].add_p2p_connection(TestNode(), send_version=False)
 
-        NetworkThread().start()  # Start up network handling in another thread
+        network_thread_start()
 
         sleep(1)
 

test/functional/p2p-versionbits-warning.py

@@ -66,7 +66,7 @@ def run_test(self):
         # Setup the p2p connection and start up the network thread.
         self.nodes[0].add_p2p_connection(TestNode())
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
 
         # Test logic begins here
         self.nodes[0].p2p.wait_for_verack()

test/functional/sendheaders.py

@@ -222,7 +222,7 @@ def run_test(self):
         # direct fetching
         test_node = self.nodes[0].add_p2p_connection(TestNode(), services=0)
 
-        NetworkThread().start() # Start up network handling in another thread
+        network_thread_start()
 
         # Test logic begins here
         inv_node.wait_for_verack()