Skip to content

Commit

Permalink
basic_host: close swarm on Close
Browse files Browse the repository at this point in the history
Using the `BasicHost` constructor transfers the ownership of the swarm.
This is similar to how using `libp2p.New` transfers the ownership of
user provided config options like `ResourceManager`, all of which are
closed on `host.Close`
  • Loading branch information
sukunrt committed Aug 10, 2024
1 parent 16a27ff commit 025fb33
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
20 changes: 9 additions & 11 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ func (cfg *Config) addTransports() ([]fx.Option, error) {
fxopts = append(fxopts, cfg.QUICReuse...)
} else {
fxopts = append(fxopts,
fx.Provide(func(key quic.StatelessResetKey, tokenGenerator quic.TokenGeneratorKey, _ *swarm.Swarm, lifecycle fx.Lifecycle) (*quicreuse.ConnManager, error) {
fx.Provide(func(key quic.StatelessResetKey, tokenGenerator quic.TokenGeneratorKey, lifecycle fx.Lifecycle) (*quicreuse.ConnManager, error) {
var opts []quicreuse.Option
if !cfg.DisableMetrics {
opts = append(opts, quicreuse.EnableMetrics(cfg.PrometheusRegisterer))
Expand Down Expand Up @@ -469,18 +469,17 @@ func (cfg *Config) NewNode() (host.Host, error) {
fx.Provide(func() event.Bus {
return eventbus.NewBus(eventbus.WithMetricsTracer(eventbus.NewMetricsTracer(eventbus.WithRegisterer(cfg.PrometheusRegisterer))))
}),
fx.Provide(func(eventBus event.Bus, lifecycle fx.Lifecycle) (*swarm.Swarm, error) {
sw, err := cfg.makeSwarm(eventBus, !cfg.DisableMetrics)
if err != nil {
return nil, err
}
lifecycle.Append(fx.StopHook(sw.Close))
return sw, nil
fx.Provide(func() crypto.PrivKey {
return cfg.PeerKey
}),
// Make sure the swarm constructor depends on the quicreuse.ConnManager.
// That way, the ConnManager will be started before the swarm, and more importantly,
// the swarm will be stopped before the ConnManager.
fx.Decorate(func(sw *swarm.Swarm, _ *quicreuse.ConnManager, lifecycle fx.Lifecycle) *swarm.Swarm {
fx.Provide(func(eventBus event.Bus, _ *quicreuse.ConnManager, lifecycle fx.Lifecycle) (*swarm.Swarm, error) {
sw, err := cfg.makeSwarm(eventBus, !cfg.DisableMetrics)
if err != nil {
return nil, err
}
lifecycle.Append(fx.Hook{
OnStart: func(context.Context) error {
// TODO: This method succeeds if listening on one address succeeds. We
Expand All @@ -491,14 +490,13 @@ func (cfg *Config) NewNode() (host.Host, error) {
return sw.Close()
},
})
return sw
return sw, nil
}),
fx.Provide(cfg.newBasicHost),
fx.Provide(func(bh *bhost.BasicHost) host.Host {
return bh
}),
fx.Provide(func(h *swarm.Swarm) peer.ID { return h.LocalPeer() }),
fx.Provide(func(h *swarm.Swarm) crypto.PrivKey { return h.Peerstore().PrivKey(h.LocalPeer()) }),
}
transportOpts, err := cfg.addTransports()
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions p2p/host/basic/basic_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,10 @@ func (h *BasicHost) Close() error {
_ = h.emitters.evtLocalProtocolsUpdated.Close()
_ = h.emitters.evtLocalAddrsUpdated.Close()

if err := h.network.Close(); err != nil {
log.Errorf("swarm close failed: %v", err)
}

h.psManager.Close()
if h.Peerstore() != nil {
h.Peerstore().Close()
Expand Down
9 changes: 8 additions & 1 deletion p2p/host/basic/basic_host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,14 @@ func TestMultipleClose(t *testing.T) {

require.NoError(t, h.Close())
require.NoError(t, h.Close())
require.NoError(t, h.Close())
h2, err := NewHost(swarmt.GenSwarm(t), nil)

Check failure on line 86 in p2p/host/basic/basic_host_test.go

View workflow job for this annotation

GitHub Actions / go-check / All

this value of err is never used (SA4006)
defer h2.Close()

Check failure on line 87 in p2p/host/basic/basic_host_test.go

View workflow job for this annotation

GitHub Actions / go-check / All

should check returned error before deferring h2.Close() (SA5001)
require.Error(t, h.Connect(context.Background(), peer.AddrInfo{ID: h2.ID(), Addrs: h2.Addrs()}))
h.Network().Peerstore().AddAddrs(h2.ID(), h2.Addrs(), peerstore.PermanentAddrTTL)
_, err = h.NewStream(context.Background(), h2.ID())
require.Error(t, err)
require.Empty(t, h.Addrs())
require.Empty(t, h.AllAddrs())
}

func TestSignedPeerRecordWithNoListenAddrs(t *testing.T) {
Expand Down

0 comments on commit 025fb33

Please sign in to comment.