Gateway fails to suggest peer connections due to race condition in join operation

[sanity]

Problem Description

Gateway fails to forward connection petitions to joining peers because it checks num_connections() == 0 before the joiner's connection has been added to the ring. This prevents peer-to-peer connections from forming in fresh networks, leaving all peers connected only to the gateway in a star topology.

Steps to Reproduce

1. Start a fresh gateway on v0.1.28
2. Start 10 peers connecting to the gateway
3. Wait for all peers to connect
4. Query network topology

Expected Behavior

Peers should receive suggestions from the gateway to connect to other peers, forming a mesh topology.

Actual Behavior

All peers remain connected only to the gateway. Gateway logs show:

WARN freenet::operations::connect: Couldn't forward connect petition, not enough connections, tx: 01K64C77APXWZKWQ520588WQR0, joiner: v6MWKgqHaDEbNQSL

Root Cause Analysis

In crates/core/src/operations/connect.rs:1028-1034, the gateway checks if it has any connections before forwarding a connect petition:

if connection_manager.num_connections() == 0 {
    tracing::warn!(
        tx = %id,
        joiner = %joiner.peer,
        "Couldn't forward connect petition, not enough connections",
    );
    return Ok(None);
}

However, this check happens during the join operation before the joiner's connection is added to the gateway's ring (which happens in crates/core/src/ring/mod.rs:220). The sequence is:

1. Peer initiates connection to gateway
2. Gateway receives join request
3. Gateway attempts to forward connect petition
4. Gateway checks num_connections() == 0 → fails here ❌
5. (Later) Connection gets added to ring via Adding connection to peer log

Evidence

Gateway logs show peer connections being created but connection suggestions failing:

INFO freenet::transport::peer_connection: PeerConnection created with persistent keep-alive task, remote: 5.9.111.215:40005
INFO freenet_core::transport::keepalive_lifecycle: Keep-alive task STARTED for connection, remote: 5.9.111.215:40005
WARN freenet::operations::connect: Couldn't forward connect petition, not enough connections, tx: 01K64C77APXWZKWQ520588WQR0, joiner: v6MWKgqHaDEbNQSL

Notably absent: Adding connection to peer messages in gateway logs, which appear in peer logs when they successfully join.

Suggested Fixes

Several approaches could resolve this:

1. Change the threshold: Check num_connections() <= 1 to exclude only the joiner itself
2. Reorder operations: Add the connection to the ring before attempting to forward connect petitions
3. Async coordination: Wait for the connection to be added before forwarding, or use a future/promise pattern
4. Fallback logic: If no existing connections, return the gateway itself as a connection suggestion

Environment

• Freenet version: v0.1.28
• Rust version: rustc 1.82.0 (f6e511eec 2024-10-15)
• OS: Linux
• Configuration: Fresh network with 1 gateway + 10 peers
• All peers connecting to gateway at 5.9.111.215:31337

Impact

This bug effectively prevents peer mesh formation in fresh Freenet networks, forcing a star topology where all traffic must flow through the gateway. This significantly impacts:

• Network resilience (single point of failure)
• Gateway load (all traffic concentrated)
• Peer discovery and connectivity
• Network scalability

Additional Context

This was discovered during v0.1.28 release validation using network visualization tools showing all peer connections terminating at the gateway with zero peer-to-peer connections.

[AI-assisted debugging and comment]

[iduartgomez]

This prevents peer-to-peer connections from forming in fresh networks, leaving all peers connected only to the gateway in a star topology.

This is a non-issue, all peers attempt to acquire more connections all the time so this wouldn't be a problem if that works correctly.

[sanity]

Response to Feedback

Thanks for the quick response! Addressing the points raised:

Regarding Periodic Connection Acquisition

@NachoDG mentioned that there should be a background task acquiring new connections periodically. However, I cannot find evidence of this task running after the initial startup phase.

What I Found

Peer Runtime: The peers have been running for 12+ hours:

• Peers 1-5: Started at 03:01-03:03 (September 27)
• Peers 6-10: Started at 03:22 (September 27)
• All still showing only gateway connections as of 15:30+

Log Analysis: Searching peer-06's logs for connection acquisition activity:

$ sudo grep -E "acquisition|maintenance" /mnt/media/freenet-peers/peers/peer-06/peer.log
[2025-09-26T22:09:27] INFO Starting aggressive connection acquisition phase (target: 25 connections)
[2025-09-27T01:01:18] INFO Starting aggressive connection acquisition phase (target: 25 connections)  
[2025-09-27T01:22:25] INFO Starting aggressive connection acquisition phase (target: 25 connections)
[2025-09-27T01:22:35] INFO Aggressive connection phase complete. Final connections: 0/25 (took 10s)

Key observation: The "aggressive connection acquisition phase" only runs once at startup for 10 seconds, then never again. After it completes with 0/25 connections, there are no further connection attempts.

Code Review

Looking at crates/core/src/node/p2p_impl.rs:

The trigger_connection_maintenance() function (lines 133-175) exists, but it's only called during the initial aggressive acquisition phase (lines 91-100). I found no periodic/interval-based scheduling of this maintenance task.

Searching for periodic execution patterns:

$ grep -E "loop.*maintain|interval.*connect|Duration.*connection" crates/core/src/node/p2p_impl.rs
# No matches found

Conclusion

After 12+ hours of runtime, peers have made zero connection attempts beyond the initial 10-second startup phase. This suggests either:

1. The periodic maintenance task doesn't exist in v0.1.28
2. It exists but isn't being triggered/scheduled properly
3. It's running but silently failing (no log evidence of this)

Setup Details

@iggiiggi The setup script is here: https://gist.github.com/sanity/6cde1ba3b64ac68565f2437148cf402c

The network consists of:

• 1 production gateway running as systemd service on 5.9.111.215:31337
• 10 peers managed by the peer-manager.sh script, each as separate systemd services
• All peers successfully connected to gateway (verified via fdev network query)
• Zero peer-to-peer connections after 12+ hours

Next Steps

If the periodic connection maintenance task is supposed to exist and run continuously, there may be a separate bug preventing it from executing. Happy to provide more logs or test scenarios to help debug this.

[AI-assisted debugging and comment]

[sanity]

@claude Can you double-check that trigger_connection_maintenance() isn't being called except during the "aggressive" phase?

[claude]

Claude finished @sanity's task —— View job

---

Analyzing trigger_connection_maintenance() usage

• Search for all calls to trigger_connection_maintenance() in the codebase
• Analyze calling patterns and contexts
• Check for periodic scheduling mechanisms
• Report findings

Findings

You're correct - NodeP2P::trigger_connection_maintenance() is only called during the aggressive startup phase and never again after that.

Evidence

1. The function is defined at: crates/core/src/node/p2p_impl.rs:134-175

2. It's only called once at: crates/core/src/node/p2p_impl.rs:96

• This call happens inside aggressive_initial_connections() method
• That method runs for a maximum of 10 seconds (line 60)
• After it completes, trigger_connection_maintenance() is never called again

3. The call pattern:

// Line 48-131: aggressive_initial_connections() method
async fn aggressive_initial_connections(&self) {
    // ...
    let max_duration = Duration::from_secs(10);  // Line 60
    
    while start.elapsed() < max_duration {
        // ...
        if stable_rounds >= 3 && current_connections > 0 {
            // Only called here during startup loop (line 96)
            if let Err(e) = self.trigger_connection_maintenance().await {
                tracing::warn!("Failed to trigger connection maintenance: {}", e);
            }
        }
    }
    // Function ends, never called again
}

Important Distinction

However, there IS a separate continuous connection maintenance system:

Ring::connection_maintenance() at crates/core/src/ring/mod.rs:357-530

• This is spawned as a background task at ring initialization (line 144-148)
• It runs continuously with a 60-second interval (line 376, CHECK_TICK_DURATION)
• It's completely different from trigger_connection_maintenance()

The two functions have similar names but serve different purposes:

• NodeP2P::trigger_connection_maintenance() - Manual peer discovery trigger (startup only)
• Ring::connection_maintenance() - Continuous background task (always running)

So your observation is correct: the NodeP2P::trigger_connection_maintenance() function only runs during the aggressive startup phase and is never invoked again afterward.

---

Job Run | Branch

[sanity]

@claude ok, but then it sounds like there is another process that should establish new connections - meaning that the original issue with no direct connections between peers remains unexplained?

[claude]

Claude finished @sanity's task —— View job

---

Investigating connection maintenance behavior

• Read Ring::connection_maintenance() implementation
• Trace peer discovery mechanism
• Identify why peer-to-peer connections aren't forming
• Connect findings to original race condition

Analysis

You're right to question this. Let me explain what I found:

The Three Connection Mechanisms

There are three separate mechanisms that should establish peer-to-peer connections:

1. Immediate post-join request (crates/core/src/operations/connect.rs:926-956):

   • When a peer connects to the gateway, it immediately sends a FindOptimalPeer request
   • This happens right after adding the gateway connection (line 938-949)

2. Join-time forwarding (crates/core/src/operations/connect.rs:1028-1035):

   • During the CheckConnectivity phase, the gateway calls forward_conn()
   • This checks if connection_manager.num_connections() == 0 - this is the race condition from the original issue

3. Continuous maintenance (crates/core/src/ring/mod.rs:357-508):

   • The Ring::connection_maintenance() task runs every 60 seconds
   • It sends FindOptimalPeer requests via acquire_new() (line 437-443)

Why All Three Are Failing

The issue is that all three mechanisms depend on the gateway having OTHER peers to suggest:

When the gateway receives a FindOptimalPeer request (connect.rs:185-228):

if this_peer == &query_target.peer {
    // Gateway is the query target
    if let Some(desirable_peer) = op_manager.ring.closest_to_location(
        *ideal_location,
        HashSet::from([joiner.peer.clone()]), // Skip the joiner itself
    ) {
        // Forward to desirable_peer
    } else {
        tracing::debug!("No desirable peer found to connect to");
        // SILENTLY FAILS - no peer-to-peer connection formed
    }
}

The closest_to_location() function (ring/mod.rs:333-355) returns None if there are no connections other than the joiner.

The Original Issue Stands

When Peer 1 joins:

• Gateway has 0 other connections
• All three mechanisms fail (no peers to suggest)
• Peer 1 remains connected only to gateway

When Peer 2 joins:

• Gateway now has 1 other connection (Peer 1)
• Mechanism Overall architecture RFC #1 (immediate request) should work - gateway should suggest Peer 1
• Mechanism NAT traversal #2 (forward_conn) may still hit the race condition you identified
• Mechanism Decentralized email proof-of-concept #3 (maintenance) should work when it runs

However, if Mechanism #1 is also failing due to timing issues (e.g., Peer 1's connection not fully added when Peer 2's immediate request arrives), then no peer-to-peer connections will ever form.

Answer to Your Question

Yes, Ring::connection_maintenance() runs continuously and should establish peer-to-peer connections. But it works by asking the gateway "who should I connect to?" - and if the gateway doesn't know about other peers (because the immediate post-join requests failed due to race conditions), then the maintenance task will also fail.

The root cause is likely: The immediate FindOptimalPeer requests sent right after joining (mechanism #1) are failing because the gateway doesn't have other peer connections registered yet. This prevents the initial "seeding" of peer-to-peer connections, which then cascades to make all three mechanisms ineffective.

Would you like me to investigate the timing of when connections are actually added to the gateway's ring to confirm this hypothesis?

---

Job Run | Branch

[sanity]

Attaching logs from the 12+ hour test run for detailed analysis: https://gist.github.com/sanity/28dea15529735342fc672e055e25c1c0

The gist includes:

• log-summary.md: Overview of test setup and key observations
• peer-06-connection-logs.txt: Last 500 lines from a peer showing connection behavior
• gateway-connection-logs.txt: Gateway logs during peer connection window showing the race condition

These logs demonstrate both issues:

1. The initial race condition preventing connection forwarding
2. The absence of periodic connection acquisition attempts after startup

[sanity]

[Ian's local Claude]

Root Cause Analysis - It's the Original Race Condition

After digging deeper into the connection flow, I can confirm the original issue description is correct. This is a race condition in the join protocol itself that creates a chicken-and-egg problem.

The Connection Flow

Here's what should happen when a peer joins:

1. Transport layer: Peer connects to gateway → PeerConnection created ✓
2. Join request: Peer sends StartJoinReq to gateway ✓
3. Forward check: Gateway calls forward_conn() to forward the request
4. CheckConnectivity: forward_conn() sends CheckConnectivity message to gateway
5. Accept decision: Gateway processes CheckConnectivity, calls should_accept()
6. Add to ring: If accepted, gateway calls add_connection() to add peer to ring

What's Actually Happening

The flow breaks at step 3:

// connect.rs:1028-1034
if connection_manager.num_connections() == 0 {
    tracing::warn!(
        tx = %id,
        joiner = %joiner.peer,
        "Couldn't forward connect petition, not enough connections",
    );
    return Ok(None);
}

Gateway logs show:

INFO PeerConnection created with persistent keep-alive task, remote: 5.9.111.215:40005
INFO Keep-alive task STARTED for connection, remote: 5.9.111.215:40005
WARN Couldn't forward connect petition, not enough connections

Crucially missing: No CheckConnectivity, no Accepting connection, no Adding connection to peer.

The Chicken-and-Egg Problem

• forward_conn() requires num_connections() > 0 to proceed
• But num_connections() only increases when add_connection() is called
• add_connection() is only called after should_accept() returns true
• should_accept() is only called when processing CheckConnectivity
• CheckConnectivity is only sent by forward_conn() if num_connections() > 0

This creates a deadlock for the first joining peer.

Why @claude's Analysis Was Incorrect

@claude suggested the gateway doesn't have connections to forward. But actually:

1. The gateway does have 10 transport-layer connections (verified via fdev network query)
2. The gateway has zero ring-layer connections (because add_connection() was never called)
3. The check at line 1028 uses connection_manager.num_connections() which counts ring-layer connections only
4. Therefore the check fails even though transport connections exist

Impact

This affects all joining peers, not just the first:

• When peer-01 joins: Gateway has 0 ring connections → rejected
• When peer-02 joins: Gateway still has 0 ring connections → rejected
• When peer-06 joins (12 hours later): Gateway still has 0 ring connections → rejected

After 12+ hours, all 10 peers remain stuck with only gateway connections at the transport layer, but the gateway's ring remains empty, so it can never suggest peers to each other.

Suggested Fix

The check at line 1028 needs to account for in-flight join requests. Options:

1. Remove the check entirely - Let forward_conn() proceed even with 0 connections
2. Check transport connections - Use a count that includes pending/transport connections
3. Special case for gateways - Gateways should always attempt to forward
4. Check > 1 instead of > 0 - Allow forwarding when there's at least the joiner

Option 1 seems safest since the downstream should_accept() logic already handles connection limits properly.

[AI-assisted debugging and comment]

[claude]

Claude finished @sanity's task —— View job

---

Validating root cause analysis

• Read repository guidelines (CLAUDE.md)
• Examine the code at connect.rs:1028-1034
• Trace the connection flow to validate the chicken-and-egg problem
• Verify the analysis and provide findings

Findings

Your analysis is correct. I've traced the complete connection flow and confirmed the race condition you identified.

The Flow When a Peer Joins

1. Peer sends StartJoinReq (handshake.rs:865-874)
2. Gateway receives and creates InboundGwJoinRequest (handshake.rs:1017-1044)
3. Gateway calls should_accept() (handshake.rs:368)
4. Gateway calls forward_conn() (handshake.rs:431-437) ⚠️ BUG IS HERE
   • Checks connection_manager.num_connections() == 0 at connect.rs:1028
   • Joiner's connection not yet in ring → check fails
   • Returns Ok(None) without forwarding
5. Gateway returns InboundConnection event (handshake.rs:460-466)
6. (Later) Connection added to ring - too late, forwarding already missed

The Chicken-and-Egg Problem

This creates a deadlock where forward_conn() requires connections, but connections are only added after forward_conn() completes. This affects every joining peer, leaving the gateway with transport connections but an empty ring.

Fix Recommendation

I agree with your Option 1 (remove the check entirely). The downstream logic in select_forward_target() (connect.rs:1071-1079) already handles the no-peers case gracefully by returning None, which then triggers handle_unforwardable_connection() at line 1067. The explicit check at lines 1028-1035 is redundant and causes this race condition.

Removing these 8 lines would:

• Allow the join protocol to complete successfully
• Add the joiner to the gateway's ring
• Enable future FindOptimalPeer requests to succeed once multiple peers are connected
• Not break any downstream logic that expects peers to be available

Would you like me to implement this fix?

---

Job Run | Branch

[sanity]

It would be useful to get @iduartgomez and @netsirius's opinion on this but in the meantime in the interests of time @claude can you implement the fix in a PR?

[claude]

Claude finished @sanity's task —— View job • claude/issue-1863-20250927-1826 • Create PR ➔

---

Implementing fix for gateway connection race condition

• Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
• Read the target file crates/core/src/operations/connect.rs
• Remove the redundant connection check (lines 1028-1035)
• Check for related tests that might need updating - No tests depend on this check
• Run linting/testing commands - Cannot run (requires approval), will be checked in CI
• Commit and push changes
• Provide PR creation link

Summary

Successfully removed the redundant num_connections() == 0 check from crates/core/src/operations/connect.rs:1028-1035 that was causing the race condition.

What was changed:

• Removed 8 lines of code that checked for zero connections before forwarding join requests
• This check was creating a chicken-and-egg problem preventing peers from joining the network
• The downstream select_forward_target() function already handles the no-peers case gracefully

Why this fix works:

• The removed check was happening before the joiner's connection was added to the ring
• This caused all join requests to fail with "not enough connections"
• Removing it allows the join protocol to complete and add connections to the ring
• Future peers can then be suggested to connect to existing peers

---

Job Run | Branch