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

host.Close() or cancelling context does not terminate listeners #992

Closed
leoluk opened this issue Aug 17, 2020 · 4 comments
Closed

host.Close() or cancelling context does not terminate listeners #992

leoluk opened this issue Aug 17, 2020 · 4 comments
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@leoluk
Copy link

leoluk commented Aug 17, 2020

Calling Close() on a libp2p.Host does not close open listener sockets.

The Host's ctx also isn't passed to the listeners, a Background context is used instead:

https://github.com/libp2p/go-libp2p-transport-upgrader/blob/0c577bb483255c5046677a49624fb45c862d616b/upgrader.go#L39

How to properly tear down a libp2p.Host?

@leoluk leoluk added the kind/bug A bug in existing code (including security flaws) label Aug 17, 2020
@leoluk leoluk changed the title host.Close() does not terminate listeners host.Close() or cancelling context does not terminate listeners Aug 18, 2020
@Stebalien
Copy link
Member

That context is closed when the listener is closed. The listeners are closed in https://github.com/libp2p/go-libp2p-swarm/blob/58c167a7b0c3294d416342d781465214d514e70f/swarm.go#L158-L164

Have you observed this issue in practice?

@leoluk
Copy link
Author

leoluk commented Aug 25, 2020

Have you observed this issue in practice?

Yes, here's the code snippet that initializes libp2p:

h, err := libp2p.New(ctx,
	// Use the keypair we generated
	libp2p.Identity(priv),

	// Multiple listen addresses
	libp2p.ListenAddrStrings(
		// Listen on QUIC only.
		fmt.Sprintf("/ip4/0.0.0.0/udp/%d/quic", *p2pPort),
		fmt.Sprintf("/ip6/::/udp/%d/quic", *p2pPort),
	),

	// Enable TLS security as the only security protocol.
	libp2p.Security(libp2ptls.ID, libp2ptls.New),

	// Enable QUIC transport as the only transport.
	libp2p.Transport(libp2pquic.NewTransport),

	// Let's prevent our peer from having too many
	// connections by attaching a connection manager.
	libp2p.ConnectionManager(connmgr.NewConnManager(
		100,         // Lowwater
		400,         // HighWater,
		time.Minute, // GracePeriod
	)),

	// Let this host use the DHT to find other hosts
	libp2p.Routing(func(h host.Host) (routing.PeerRouting, error) {
		idht, err = dht.New(ctx, h, dht.Mode(dht.ModeServer),
			dht.ProtocolPrefix(protocol.ID("/"+*p2pNetworkID)),
		)
		return idht, err
	}),
)

if err != nil {
	panic(err)
}

defer func() {
	h.Close()
}()

Neither cancelling ctx, nor calling h.Close() seems to close listener sockets and we have to restart the whole process.

@Stebalien
Copy link
Member

Ah, interesting. That's actually an issue with the QUIC transport. We reference-count and share udp sockets in the QUIC transport, closing them only after they've been idle for at least 10 seconds.

I'm not sure what the correct fix here is, unfortunately. We don't have way to "close" transports, just listeners and connections.

@Stebalien
Copy link
Member

Closing as libp2p/go-libp2p-quic-transport#211 has been merged and this will be fixed in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

2 participants