Skip to content

Commit

Permalink
fix autonat race
Browse files Browse the repository at this point in the history
fix #7947
move `BasicHost.AutoNat` to a private field (it has no public method and shouldn't be accessed afaik.
Instead add a setter for config that sets it while holding the address mutex to prevent reads of the
field at the same time.
  • Loading branch information
willscott committed Feb 27, 2021
1 parent bbde01b commit c2c5918
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 5 deletions.
4 changes: 3 additions & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,12 @@ func (cfg *Config) NewNode(ctx context.Context) (host.Host, error) {
autonatOpts = append(autonatOpts, autonat.WithReachability(*cfg.AutoNATConfig.ForceReachability))
}

if h.AutoNat, err = autonat.New(ctx, h, autonatOpts...); err != nil {
autonat, err := autonat.New(ctx, h, autonatOpts...)
if err != nil {
h.Close()
return nil, fmt.Errorf("cannot enable autorelay; autonat failed to start: %v", err)
}
h.SetAutoNat(autonat)

// start the host background tasks
h.Start()
Expand Down
16 changes: 13 additions & 3 deletions p2p/host/basic/basic_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ type BasicHost struct {
signKey crypto.PrivKey
caBook peerstore.CertifiedAddrBook

AutoNat autonat.AutoNAT
autoNat autonat.AutoNAT
}

var _ host.Host = (*BasicHost)(nil)
Expand Down Expand Up @@ -811,6 +811,7 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr {
h.addrMu.RLock()
filteredIfaceAddrs := h.filteredInterfaceAddrs
allIfaceAddrs := h.allInterfaceAddrs
autonat := h.autoNat
h.addrMu.RUnlock()

// Iterate over all _unresolved_ listen addresses, resolving our primary
Expand All @@ -831,8 +832,8 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr {
// but not have an external network card,
// so net.InterfaceAddrs() not has the public ip
// The host can indeed be dialed !!!
if h.AutoNat != nil {
publicAddr, _ := h.AutoNat.PublicAddr()
if autonat != nil {
publicAddr, _ := autonat.PublicAddr()
if publicAddr != nil {
finalAddrs = append(finalAddrs, publicAddr)
}
Expand Down Expand Up @@ -992,6 +993,15 @@ func (h *BasicHost) AllAddrs() []ma.Multiaddr {
return dedupAddrs(finalAddrs)
}

// SetAutoNat sets the autonat service for the host.
func (h *BasicHost) SetAutoNat(a autonat.AutoNAT) {
h.addrMu.Lock()
defer h.addrMu.Unlock()
if h.autoNat == nil {
h.autoNat = a
}
}

// Close shuts down the Host's services (network, etc).
func (h *BasicHost) Close() error {
h.closeSync.Do(func() {
Expand Down
2 changes: 1 addition & 1 deletion p2p/host/basic/basic_host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func TestHostAddrsFactory(t *testing.T) {
}

var err error
h.AutoNat, err = autonat.New(ctx, h, autonat.WithReachability(network.ReachabilityPublic))
h.autoNat, err = autonat.New(ctx, h, autonat.WithReachability(network.ReachabilityPublic))
if err != nil {
t.Fatalf("should be able to attach autonat: %v", err)
}
Expand Down

0 comments on commit c2c5918

Please sign in to comment.