streamWrapper.Close() can block indefinitely when the remote peer is slow
or unresponsive during the multistream-select handshake completion.
The lazy multistream protocol negotiation defers reading the handshake
response until Close() is called. If the remote peer doesn't respond,
the read blocks forever, causing goroutine leaks.
This is particularly problematic for bitswap servers where taskWorkers
can get stuck trying to close streams after sending blocks.
The fix sets a read deadline (using DefaultNegotiationTimeout) before
calling the multistream Close(), ensuring the operation will time out
rather than block indefinitely.
Related: https://github.com/multiformats/go-multistream/issues/47
Related: https://github.com/multiformats/go-multistream/pull/48
Providing unknown addresses to the user, helps them infer whether autonatv2 has any more addresses it might confirm.
There's no upside to not sending this information.
This also changes host.ConfirmedAddrs to return all the three categories, Reachable, Unreachable, and Unknown addrs.
This introduces addrsReachabilityTracker that tracks reachability on
a set of addresses. It probes reachability for addresses periodically
and has an exponential backoff in case there are too many errors
or we don't have any valid autonatv2 peer.
There's no smartness in the address selection logic currently. We just
test all provided addresses. It also doesn't use the addresses provided
by `AddrsFactory`, so currently there's no way to get a user provided
address tested for reachability, something that would be a problem for
dns addresses. I intend to introduce an alternative to
`AddrsFactory`, something like, `AnnounceAddrs(addrs []ma.Multiaddr)`
that's just appended to the set of addresses that we have, and check
reachability for those addresses.
There's only one method exposed in the BasicHost right now that's
`ReachableAddrs() []ma.Multiadd`r that returns the host's reachable
addrs. Users can also use the event `EvtHostReachableAddrsChanged`
to be notified when any addrs reachability changes.
* autonat: fix interaction with autorelay
* Fix race in test
* Use deadline from context if available for DialBack
* Return hasNewAddrs correctly
* nit: cleanup contains check
* Shuffle peers
* nits
* Change comment to indicate the bug
* holepuncher: pass address function in constructor (#2979)
* holepunch: pass address function in constructor
* nit
* Remove getPublicAddrs
---------
Co-authored-by: Marco Munizaga <git@marcopolo.io>
* Make a copy of the multiaddr slice in Addrs()
---------
Co-authored-by: Marco Munizaga <git@marcopolo.io>
* Remove unused resolver in basic host
* Refactor Swarm.resolveAddrs
Refactors how DNS Address resolution works.
* lint
* Move MultiaddrDNSResolver interface to core
* Reserve output space for addresses left to resolve
* feat: core/transport: Add SkipResolver interface (#2989)
* Rebase on top of resolveAddrs refactor
* Add comments
* Sanitize address inputs when returning a reservation message (#3006)
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`
* Remove this hack, the new observed address manager just works
* Infer addresses that share the same local thin waist
* Use Interface listen address as well
Rename the Transient state on connection to Limited. This is more appropriate and also
doesn't conflict with the transient resource manager scope.
Adds a Limited connectedness state for peers connected to us over Limited connections.
This allows users to ignore such peers if they are interested in only peers connected to
us over Unlimited connections.
For some peers who disconnect before we sent a Connectedness event, we will now
only send a Disconnected event.
---------
Co-authored-by: guillaumemichel <guillaume@michel.id>
Co-authored-by: sukun <sukunrt@gmail.com>
Co-authored-by: Marco Munizaga <git@marcopolo.io>
* config: refactor AutoNAT construction into separate method
* config: use a lifecycle hook to start listening on swarm addresses
* use Fx to construct the host
* add a test for constructing a routed host
* use Fx hooks to start the host
* config: use Fx lifecycle hooks to start AutoRelay and for PeerRouting
* basichost: don't close the swarm
The swarm is not constructed by the basic host, thus is shouldn't be
closed by it.
* config: use Fx hook to close the quicreuse connection manager
* test for goroutine leaks when starting/stopping fx
To do this, I've had to move a few leaky tests into a separate package.
I've filed a bug for the AutoNAT issue (#2743) but the "error on
startup" issue is going to require some pretty invasive changes (we need
to construct _then_ start).
* go fmt
* Ignore one more top function
* Typo
* Ignore any not top
---------
Co-authored-by: Sukun <sukunrt@gmail.com>
Co-authored-by: Steven Allen <steven@stebalien.com>
Co-authored-by: Marco Munizaga <git@marcopolo.io>
This does not do change anything because the transport always returns the same multiaddr you passed in in case of failure, it just looks wrong and relies on a obscure detail of the webtransport's AddCertHashes function, while this does not.
Marcin Rataj