Skip to content

Commit 34e08b3

Browse files
committed
[tests] Fix network threading in functional tests
assumevalid.py, example_test.py and p2p-acceptblocks.py add p2p_connections after the NetworkThread has been started. This isn't permitted. Fix test to restart the network thread when adding new connections. p2p-leaktest.py had a potential race condition if the NetworkThread hadn't terminated by the time we tried to restart it.
1 parent 74e64f2 commit 34e08b3

File tree

4 files changed

+31
-8
lines changed

4 files changed

+31
-8
lines changed

test/functional/assumevalid.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@
3838
CTransaction,
3939
CTxIn,
4040
CTxOut,
41+
network_thread_join,
42+
network_thread_start,
4143
P2PInterface,
4244
msg_block,
43-
msg_headers,
44-
network_thread_start)
45+
msg_headers)
4546
from test_framework.script import (CScript, OP_TRUE)
4647
from test_framework.test_framework import BitcoinTestFramework
4748
from test_framework.util import assert_equal
@@ -159,13 +160,22 @@ def run_test(self):
159160
self.block_time += 1
160161
height += 1
161162

163+
# We're adding new connections so terminate the network thread
164+
self.nodes[0].disconnect_p2ps()
165+
network_thread_join()
166+
162167
# Start node1 and node2 with assumevalid so they accept a block with a bad signature.
163168
self.start_node(1, extra_args=["-assumevalid=" + hex(block102.sha256)])
164-
p2p1 = self.nodes[1].add_p2p_connection(BaseNode())
165-
p2p1.wait_for_verack()
166-
167169
self.start_node(2, extra_args=["-assumevalid=" + hex(block102.sha256)])
170+
171+
p2p0 = self.nodes[0].add_p2p_connection(BaseNode())
172+
p2p1 = self.nodes[1].add_p2p_connection(BaseNode())
168173
p2p2 = self.nodes[2].add_p2p_connection(BaseNode())
174+
175+
network_thread_start()
176+
177+
p2p0.wait_for_verack()
178+
p2p1.wait_for_verack()
169179
p2p2.wait_for_verack()
170180

171181
# send header lists to all three nodes

test/functional/example_test.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
mininode_lock,
2222
msg_block,
2323
msg_getdata,
24+
network_thread_join,
2425
network_thread_start,
2526
)
2627
from test_framework.test_framework import BitcoinTestFramework
@@ -131,7 +132,7 @@ def custom_method(self):
131132
def run_test(self):
132133
"""Main test logic"""
133134

134-
# Create a P2P connection to one of the nodes
135+
# Create P2P connections to two of the nodes
135136
self.nodes[0].add_p2p_connection(BaseNode())
136137

137138
# Start up network handling in another thread. This needs to be called
@@ -188,7 +189,14 @@ def run_test(self):
188189
connect_nodes(self.nodes[1], 2)
189190

190191
self.log.info("Add P2P connection to node2")
192+
# We can't add additional P2P connections once the network thread has started. Disconnect the connection
193+
# to node0, wait for the network thread to terminate, then connect to node2. This is specific to
194+
# the current implementation of the network thread and may be improved in future.
195+
self.nodes[0].disconnect_p2ps()
196+
network_thread_join()
197+
191198
self.nodes[2].add_p2p_connection(BaseNode())
199+
network_thread_start()
192200
self.nodes[2].p2p.wait_for_verack()
193201

194202
self.log.info("Wait for node2 reach current tip. Test that it has propagated all the blocks to us")

test/functional/p2p-acceptblock.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,13 @@ def run_test(self):
207207
# disconnect/reconnect first
208208

209209
self.nodes[0].disconnect_p2ps()
210-
test_node = self.nodes[0].add_p2p_connection(P2PInterface())
210+
self.nodes[1].disconnect_p2ps()
211+
network_thread_join()
211212

213+
test_node = self.nodes[0].add_p2p_connection(P2PInterface())
214+
network_thread_start()
212215
test_node.wait_for_verack()
216+
213217
test_node.send_message(msg_block(block_h1f))
214218

215219
test_node.sync_with_ping()

test/functional/p2p-leaktests.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,9 @@ def run_test(self):
126126

127127
self.nodes[0].disconnect_p2ps()
128128

129-
# Wait until all connections are closed
129+
# Wait until all connections are closed and the network thread has terminated
130130
wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 0)
131+
network_thread_join()
131132

132133
# Make sure no unexpected messages came in
133134
assert(no_version_bannode.unexpected_msg == False)

0 commit comments

Comments
 (0)