Skip to content

Commit 39c79bc

Browse files
committed
Use deterministic port allocation in integration tests
Integration tests allocate listening ports for test nodes by binding to 127.0.0.1:0, reading the OS-assigned port, then immediately dropping the socket. Later, Node::start() tries to bind to that same port. This has two problems: Parallel test collisions: with 35 tests running concurrently, each creating 2-4 nodes with 2 ports each, the OS can reassign a freed port to another test's node before the original node calls start(). This is a classic TOCTOU race. In CI, it caused ~50% of Rust test runs to fail with InvalidSocketAddress, always exactly one random test per run. Restart instability: when a test stops and restarts a node, the port is released during stop() and must be re-acquired during start(). Another test's node can grab it in between. The peer store still has the old address, so auto-reconnection also fails. Both problems are inherent to any scheme that allocates a port and later releases it. The only way to guarantee a port stays yours is to never release it, or to never share the port space. A deterministic atomic counter starting at a fixed base port (20000) solves both problems: each fetch_add returns a unique value, so no two nodes in the same process ever get the same port, and ports are stable across restarts because the same node keeps the same config. There is no external contention because CI runners are isolated. AI tools were used in preparing this commit.
1 parent a555133 commit 39c79bc

File tree

2 files changed

+14
-16
lines changed

2 files changed

+14
-16
lines changed

tests/common/mod.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::collections::{HashMap, HashSet};
1414
use std::env;
1515
use std::future::Future;
1616
use std::path::PathBuf;
17+
use std::sync::atomic::{AtomicU16, Ordering};
1718
use std::sync::{Arc, RwLock};
1819
use std::time::Duration;
1920

@@ -268,17 +269,14 @@ pub(crate) fn random_storage_path() -> PathBuf {
268269
temp_path
269270
}
270271

271-
pub(crate) fn random_listening_addresses() -> Vec<SocketAddress> {
272-
let num_addresses = 2;
273-
let mut listening_addresses = HashSet::new();
272+
static NEXT_PORT: AtomicU16 = AtomicU16::new(20000);
274273

275-
while listening_addresses.len() < num_addresses {
276-
let socket = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
277-
let address: SocketAddress = socket.local_addr().unwrap().into();
278-
listening_addresses.insert(address);
279-
}
280-
281-
listening_addresses.into_iter().collect()
274+
pub(crate) fn generate_listening_addresses() -> Vec<SocketAddress> {
275+
let port = NEXT_PORT.fetch_add(2, Ordering::Relaxed);
276+
vec![
277+
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port },
278+
SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: port + 1 },
279+
]
282280
}
283281

284282
pub(crate) fn random_node_alias() -> Option<NodeAlias> {
@@ -304,7 +302,7 @@ pub(crate) fn random_config(anchor_channels: bool) -> TestConfig {
304302
println!("Setting random LDK storage dir: {}", rand_dir.display());
305303
node_config.storage_dir_path = rand_dir.to_str().unwrap().to_owned();
306304

307-
let rand_listening_addresses = random_listening_addresses();
305+
let rand_listening_addresses = generate_listening_addresses();
308306
println!("Setting random LDK listening addresses: {:?}", rand_listening_addresses);
309307
node_config.listening_addresses = Some(rand_listening_addresses);
310308

tests/integration_tests_rust.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ use common::{
2121
expect_channel_pending_event, expect_channel_ready_event, expect_channel_ready_events,
2222
expect_event, expect_payment_claimable_event, expect_payment_received_event,
2323
expect_payment_successful_event, expect_splice_pending_event, generate_blocks_and_wait,
24-
open_channel, open_channel_push_amt, open_channel_with_all, premine_and_distribute_funds,
25-
premine_blocks, prepare_rbf, random_chain_source, random_config, random_listening_addresses,
24+
generate_listening_addresses, open_channel, open_channel_push_amt, open_channel_with_all,
25+
premine_and_distribute_funds, premine_blocks, prepare_rbf, random_chain_source, random_config,
2626
setup_bitcoind_and_electrsd, setup_builder, setup_node, setup_two_nodes, splice_in_with_all,
2727
wait_for_tx, TestChainSource, TestStoreType, TestSyncStore,
2828
};
@@ -1429,9 +1429,9 @@ async fn test_node_announcement_propagation() {
14291429
node_a_alias_bytes[..node_a_alias_string.as_bytes().len()]
14301430
.copy_from_slice(node_a_alias_string.as_bytes());
14311431
let node_a_node_alias = Some(NodeAlias(node_a_alias_bytes));
1432-
let node_a_announcement_addresses = random_listening_addresses();
1432+
let node_a_announcement_addresses = generate_listening_addresses();
14331433
config_a.node_config.node_alias = node_a_node_alias.clone();
1434-
config_a.node_config.listening_addresses = Some(random_listening_addresses());
1434+
config_a.node_config.listening_addresses = Some(generate_listening_addresses());
14351435
config_a.node_config.announcement_addresses = Some(node_a_announcement_addresses.clone());
14361436

14371437
// Node B will only use listening addresses
@@ -1441,7 +1441,7 @@ async fn test_node_announcement_propagation() {
14411441
node_b_alias_bytes[..node_b_alias_string.as_bytes().len()]
14421442
.copy_from_slice(node_b_alias_string.as_bytes());
14431443
let node_b_node_alias = Some(NodeAlias(node_b_alias_bytes));
1444-
let node_b_listening_addresses = random_listening_addresses();
1444+
let node_b_listening_addresses = generate_listening_addresses();
14451445
config_b.node_config.node_alias = node_b_node_alias.clone();
14461446
config_b.node_config.listening_addresses = Some(node_b_listening_addresses.clone());
14471447
config_b.node_config.announcement_addresses = None;

0 commit comments

Comments
 (0)