libp2p / libp2p/go-libp2p

dial limiting is racy

Open
#1,105 7 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs/triage
Dominant language
Go
Stars
6.9k
Forks
1.3k
Avg merge
13d 21h
Merged PRs (30d)
1

Description

In tests we're seeing "Application Error 0x0" quite frequently, for example in the `TestNewDialOld`:
https://github.com/libp2p/go-libp2p/blob/a26ef706573a16ec04d2f0f2c3dcaab70c27fe40/p2p/host/basic/basic_host_test.go#L452-L484
The error occurs on the `Write` call. This is the error we'd expect from QUIC when a connection is closed by the application via `transport.Conn.Close()`.

From the server's perspective, this is inherently racy. Consider the following packet flow, visualizing the 3-way handshakes that both QUIC and TCP perform when setting up a connection.
image
From the client's perspective, the TCP handshake finishes first, so it decides to use the TCP connection, and cancel the QUIC handshake. As the handshakes are run in separate threats, there's no guarantee that the QUIC handshake is canceled before QUIC's TLS Finished message is sent out, so it might happen that the QUIC connection is closed after the handshake completed.
From the server's perspective, the QUIC handshake finishes first, and it starts using the connection, only to notice that the connection is closed a short time later (which then causes the `Write` on the stream to error).

The fact that in practice TCP needs another round-trip for the cryptographic handshake means that we have to fit another round-trip in here for TCP. For the situation described above to occur, the RTTs experienced by QUIC and TCP need to conspire to make this sequence of events possible. This is unlikely and probably the reason we don't see this problem a lot more frequently.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with TestNewDialOld in p2p/host/basic/basic_host_test.go at the linked lines and inspect the Write call that reports the QUIC error. Trace the TCP and QUIC handshake cancellation around transport.Conn.Close(), then define a testable completion condition for the race and verify it with the relevant basic host tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.