Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iroh-gossip fails to establish connection if it is established simultaneously from both sides #2307

Closed
link2xt opened this issue May 19, 2024 · 0 comments · Fixed by #2318
Closed
Assignees
Labels
bug Something isn't working c-iroh-gossip
Milestone

Comments

@link2xt
Copy link
Contributor

link2xt commented May 19, 2024

Here if we get incoming connection and already have a connection with the same peer, old connection is dropped:

self.conns.insert(peer_id, conn.clone());

This is fine because connection is just a reference and it got cloned into connection_loop already.

But here we create new send_tx and evict existing one from the map:

self.conn_send_tx.insert(peer_id, send_tx.clone());

Previously started connection loop detects that its send_rx side got closed and breaks:

None => break,

Then existing connection gets dropped and only incoming connection remains.

If two peers try to establish connection to each other simultaneously, this is what happens. First, node establishes outgoing connection. Then it gets incoming connection and closes its outgoing connection because send_rx got closed. At this point "connection closed without error" is printed.

Because this happens on the other side, the other side also closes its outgoing connection. For the first peer it is incoming connection. When it detects that incoming connection got closed, it logs

iroh_gossip::net: connection closed with error connection lost

Caused by:
    closed by peer: 0

Then both sides have no connection anymore.

Not accepting incoming connection does not work either, then we have two outgoing connection and don't listen on the incoming ones. I think we should just keep incoming connection even if it is replaced (send_tx gets dropped) until it is closed by the other side, e.g. because it leaves gossip explicitly.

This is my analysis of what happens in deltachat/deltachat-core-rust#5595

@dignifiedquire dignifiedquire added bug Something isn't working c-iroh-gossip labels May 19, 2024
@dignifiedquire dignifiedquire added this to the v0.17.0 milestone May 19, 2024
@ramfox ramfox moved this to 📋 Backlog in iroh May 21, 2024
@ramfox ramfox moved this from 📋 Backlog to 🏗 In progress in iroh May 22, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh May 22, 2024
rklaehn pushed a commit to n0-computer/iroh-blobs that referenced this issue Oct 22, 2024
dignifiedquire pushed a commit to n0-computer/iroh-gossip that referenced this issue Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c-iroh-gossip
Projects
Archived in project
3 participants