test: Fix all races after a socket is closed gracefully
What the developer wrote
Authored by MarcoFalke
test: Fix all races after a socket is closed gracefully
This waits for any disconnect (e.g. from a restart of one of the nodes)
to fully happen before the next connect.
Can be reviewed with the git option:
--color-moved=dimmed-zebra
What changed, and why it matters
This change only touches Bitcoin Core's internal Python test framework. It makes a test helper wait for an old peer connection to fully disappear before creating a new one, reducing flaky test failures. It does not change the actual Bitcoin node software that users run, so it has no direct security impact on the network or on users.
No security action required. Treat as a normal test reliability improvement.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit modifies test_framework.py’s connect_nodes_p2p helper. Previously it only waited for the new outbound/inbound peer entries to appear in getpeerinfo after calling addconnection. The patch moves the subversion-based peer lookup earlier and adds wait_until loops that ensure any prior connection between the two nodes has been removed from getpeerinfo before the new addconnection is issued. This closes a race window where a graceful disconnect from a prior test step (e.g. node restart) had not yet completed, which could cause intermittent test failures. The change is purely in functional-test infrastructure.
Changed components
test/functional/test_framework/test_framework.pyInspect captured patch +11 / −7
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index b809e68f..229ff893 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -559,6 +559,17 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
"""
from_connection = self.nodes[a]
to_connection = self.nodes[b]
+
+ # Use subversion as peer id. Test nodes have their node number appended to the user agent string
+ from_connection_subver = from_connection.getnetworkinfo()['subversion']
+ to_connection_subver = to_connection.getnetworkinfo()['subversion']
+
+ def find_conn(node, peer_subversion, inbound):
+ return next(filter(lambda peer: peer['subver'] == peer_subversion and peer['inbound'] == inbound, node.getpeerinfo()), None)
+
+ self.wait_until(lambda: not find_conn(from_connection, to_connection_subver, inbound=False))
+ self.wait_until(lambda: not find_conn(to_connection, from_connection_subver, inbound=True))
+
ip_port = "127.0.0.1:" + str(p2p_port(b))
if peer_advertises_v2 is None:
@@ -574,13 +585,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
if not wait_for_connect:
return
- # Use subversion as peer id. Test nodes have their node number appended to the user agent string
- from_connection_subver = from_connection.getnetworkinfo()['subversion']
- to_connection_subver = to_connection.getnetworkinfo()['subversion']
-
- def find_conn(node, peer_subversion, inbound):
- return next(filter(lambda peer: peer['subver'] == peer_subversion and peer['inbound'] == inbound, node.getpeerinfo()), None)
-
self.wait_until(lambda: find_conn(from_connection, to_connection_subver, inbound=False) is not None)
self.wait_until(lambda: find_conn(to_connection, from_connection_subver, inbound=True) is not None)
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.