Skip to content

Commit

Permalink
multi: Handle detected data race conditions
Browse files Browse the repository at this point in the history
All instances of data race are related to accessing
KnownAddress elements, except for peer.go where its
related to closure.

Following are the changes:
- Add KnownAddress.mtx (sync.Mutex) and used same to
 sync access points
- For peer.go, pass local copy to the closure
  • Loading branch information
maicalal committed Nov 30, 2017
1 parent 664ccfc commit ad33b0e
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 5 deletions.
5 changes: 5 additions & 0 deletions addrmgr/addrmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,9 @@ func (a *AddrManager) updateAddress(netAddr, srcAddr *wire.NetAddress) {
naCopy := *ka.na
naCopy.Timestamp = netAddr.Timestamp
naCopy.AddService(netAddr.Services)
ka.mtx.Lock()
ka.na = &naCopy
ka.mtx.Unlock()
}

// If already in tried, we have nothing to do here.
Expand Down Expand Up @@ -823,9 +825,12 @@ func (a *AddrManager) Attempt(addr *wire.NetAddress) {
if ka == nil {
return
}

// set last tried time to now
ka.mtx.Lock()
ka.attempts++
ka.lastattempt = time.Now()
ka.mtx.Unlock()
}

// Connected Marks the given address as currently connected and working at the
Expand Down
10 changes: 10 additions & 0 deletions addrmgr/knownaddress.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package addrmgr

import (
"sync"
"time"

"github.com/decred/dcrd/wire"
Expand All @@ -14,6 +15,7 @@ import (
// KnownAddress tracks information about a known network address that is used
// to determine how viable an address is.
type KnownAddress struct {
mtx sync.Mutex
na *wire.NetAddress
srcAddr *wire.NetAddress
attempts int
Expand All @@ -26,18 +28,24 @@ type KnownAddress struct {
// NetAddress returns the underlying wire.NetAddress associated with the
// known address.
func (ka *KnownAddress) NetAddress() *wire.NetAddress {
ka.mtx.Lock()
defer ka.mtx.Unlock()
return ka.na
}

// LastAttempt returns the last time the known address was attempted.
func (ka *KnownAddress) LastAttempt() time.Time {
ka.mtx.Lock()
defer ka.mtx.Unlock()
return ka.lastattempt
}

// chance returns the selection probability for a known address. The priority
// depends upon how recently the address has been seen, how recently it was last
// attempted and how often attempts to connect to it have failed.
func (ka *KnownAddress) chance() float64 {
ka.mtx.Lock()
defer ka.mtx.Unlock()
now := time.Now()
lastSeen := now.Sub(ka.na.Timestamp)
lastAttempt := now.Sub(ka.lastattempt)
Expand Down Expand Up @@ -73,6 +81,8 @@ func (ka *KnownAddress) chance() float64 {
// All addresses that meet these criteria are assumed to be worthless and not
// worth keeping hold of.
func (ka *KnownAddress) isBad() bool {
ka.mtx.Lock()
defer ka.mtx.Unlock()
if ka.lastattempt.After(time.Now().Add(-1 * time.Minute)) {
return false
}
Expand Down
10 changes: 5 additions & 5 deletions peer/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1861,12 +1861,12 @@ func (p *Peer) AssociateConnection(conn net.Conn) {
p.na = na
}

go func() {
if err := p.start(); err != nil {
log.Debugf("Cannot start peer %v: %v", p, err)
p.Disconnect()
go func(peer *Peer) {
if err := peer.start(); err != nil {
log.Debugf("Cannot start peer %v: %v", peer, err)
peer.Disconnect()
}
}()
}(p)
}

// Connected returns whether or not the peer is currently connected.
Expand Down

0 comments on commit ad33b0e

Please sign in to comment.