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

[Proposal] .Context() for network.Conn (or maybe just swarm.Conn) #3116

Open
MarcoPolo opened this issue Dec 19, 2024 · 0 comments
Open

[Proposal] .Context() for network.Conn (or maybe just swarm.Conn) #3116

MarcoPolo opened this issue Dec 19, 2024 · 0 comments

Comments

@MarcoPolo
Copy link
Collaborator

The proposal is for conn.Context to return a context that is cancelled when the connection is closed. User's can use the context to cleanup resources with context.Afterfunc or as a context for some other work.

Motivation

It's important for users to clean up resources associated with a connection when that connection is closed. This is important for DoS resiliency and for performance. Right now the only way to do so is by registering a network.Notifiee, but that is relatively unergonomic.

There are network events, but those currently only tell you if you are (dis)connected to a peer, not if the connection you are referencing is closed.

Consider this common pattern seen in go-libp2p-kad-dht and boxo:

  • Create a stream. This stream is implicitly tied to a connection.
  • Store a reference to the stream to reuse the stream for multiple messages.
  • subscribe to event.EvtPeerConnectednessChanged events to remove the
    stream reference when a peer disconnects.

This pattern is subtly incorrect. Imagine you had two connections to a peer, opened a stream on connection A, and then had the remote close that connection. You are still connected, but you haven't cleaned up your stream reference. This will prevent the GC from freeing the memory associated with that connection.

Another use case is to signal early cancellation of some work. For example, autonatv2 could use this to cancel it's dial back work if the connection is closed. Since, if the connection is closed, there's no way to report back the results of the dial back.

Counter arguments

  • Don't reuse streams and only keep well scoped references to a stream.
  • It introduces another way to of cleaning up resources.
    • This is true. I'd argue this is better than using network.Notifiee. And, judging from existing usages, it's not often used.

Prior Art

  • In the Go standard library, *http.Request and *tls.ClientHelloInfo both have this method.
  • In quic-go, the quic.Connection interface has this method.

Details

As an implementation detail, the returned context will implement it's own AfterFunc method that does not spawn a goroutine. Users can still spawn their own goroutine if needed, but for the common case of dropping some state quickly it would be nice to avoid that overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant