use Fx to start and stop the host, swarm, autorelay and quicreuse

[marten-seemann]

Fixes #2117. Part of #1993. Part of #2514.

[MarcoPolo]

autorelay depends on Identify to start first, how can we enforce that?

[marten-seemann]

I think this is guaranteed because we construct the host first, then autorelay (since the autorelay constructor depends on the host). Fx executes startup hooks in the order they were added, so this determines that host.Start is called before autorelay.Start. Now I admit that this is pretty subtle.

The correct solution is to remove the IDService() from the host, and instead pass a reference to identify to the autorelay constructor. That would make the dependency tree more obvious. Let's do that in a follow-up PR?

[MarcoPolo]

Are we closing the swarm twice?

[MarcoPolo]

As mentioned elsewhere, we are but that's okay since the swarm close is guarded to only close once. The closing twice is useful because each close is logically for a different reason.

1. Here we are closing because we created the swarm.
2. Below, in decorate we are closing to ensure we close before the quicreuse conn manager.

By closing twice we prevent these different procedures from being tangled.

[sukunrt]

I think you can have just one type which uses host.Host interface instead of a separate type for closableBasicHost and closableRoutedHost

type closableHost struct {
	*fx.App
	host.Host
}

[sukunrt]

Will this call sw.Close twice? One for this and one for the previous option where we do,

	lifecycle.Append(fx.StopHook(sw.Close))

I think this is fine though, swarm.Close is guarded by a sync.Once

[sukunrt]

Thanks for removing AutoRelayHost 🎉

[MarcoPolo]

We need to add a couple more tests. At least:

1. A test that asserts the connmanager is closed after we close the host/swarm.
2. A test that asserts their are no dangling goroutines after starting and stopping a host.

[MarcoPolo]

As mentioned elsewhere, we are but that's okay since the swarm close is guarded to only close once. The closing twice is useful because each close is logically for a different reason.

1. Here we are closing because we created the swarm.
2. Below, in decorate we are closing to ensure we close before the quicreuse conn manager.

By closing twice we prevent these different procedures from being tangled.

[Stebalien]

I've added a dangling goroutine test which should be sufficient. It implicitly asserts that the connection manager is closed as the connection manager launches a background goroutine. Ideally we'd have some way to also assert that there are no scheduled timers, but I don't think we can.

When doing that, I noticed two issues:

1. AutoNAT fails to close its peerstore #2743.
2. We don't shut anything down if we fail to start. But, IMO, that's not something to solve in this patch.

(I'm trying to push this forward because I don't want it to develop a bunch of merge conflicts)

[MarcoPolo]

Thanks @Stebalien! I didn't know about uber/goleak. Looks helpful. I'll try to fix the current leaks.

[Stebalien]

Hm. That goroutine should have been excluded...

[Stebalien]

Oh, different nat goroutine.

[Stebalien]

I'd add github.com/jackpal/go-nat-pmp.(*Client).GetExternalAddress to the exclusion list.

[MarcoPolo]

A test that asserts the connmanager is closed after we close the host/swarm.

This is effectively covered by checking for dangling goroutines.