Multicast: detected and removed data race condition

uaalto · uaalto · commit 5945b7ee7b0e · 2015-07-13T14:59:32.000+02:00
diff --git a/src/github.com/getlantern/flashlight/localdiscovery/localdiscovery.go b/src/github.com/getlantern/flashlight/localdiscovery/localdiscovery.go
@@ -12,7 +12,6 @@ import (
 
 const (
 	messageType  = `LocalDiscovery`
-	updatePeriod = 10
 )
 
 var (
@@ -52,7 +51,7 @@ func Start(advertise bool, portToAdvertise string) {
 	mc = multicast.JoinMulticast(addOrRemoveCb, addOrRemoveCb)
 
 	if advertise {
-		mc.Payload = portToAdvertise
+		mc.SetPayload(portToAdvertise)
 		mc.StartMulticast()
 	}
 
diff --git a/src/github.com/getlantern/multicast/cmd/multicast.go b/src/github.com/getlantern/multicast/cmd/multicast.go
@@ -25,7 +25,7 @@ func main() {
 			log.Fatal("Error setting up socket for multicast loop")
 		}
 	*/
-	mc.Period = 1
+	mc.SetPeriod(1)
 	mc.StartMulticast()
 	mc.ListenPeers()
 
diff --git a/src/github.com/getlantern/multicast/multicast.go b/src/github.com/getlantern/multicast/multicast.go
@@ -30,9 +30,8 @@ type eventCallback func(string, []PeerInfo)
 
 // Multicast servent main structure
 type Multicast struct {
-	Period  int    // multicast period (in secs, 10 by default)
-	Payload string // Will be appended to the messages
-
+	period  int    // multicast period (in secs, 10 by default)
+	payload string // Will be appended to the messages
 	conn               *net.UDPConn
 	addr               *net.UDPAddr
 	failedTime         int           // timeout for peers' hello messages, 0 means no timeout
@@ -85,17 +84,33 @@ func JoinMulticast(addPeerCallback eventCallback, removePeerCallback eventCallba
 	}
 
 	return &Multicast{
-		Period:             defaultPeriod,
+		period:             defaultPeriod,
 		conn:               c,
 		addr:               udpAddr,
 		failedTime:         defaultFailedTime,
 		addPeerCallback:    aCb,
 		removePeerCallback: rCb,
+		helloTicker:        time.NewTicker(time.Duration(defaultPeriod) * time.Second),
 		quit:               make(chan bool, 1),
 		peers:              make(map[string]PeerInfo),
 	}
 }
 
+// SetPeriod will define a different interval for the periodic multicasts. Currently setting the
+// period once StartMulticast has been called is not supported, so this function must be called
+// always before StartMulticast()
+func (mc *Multicast) SetPeriod(period int) {
+	mc.period = period
+	mc.helloTicker = time.NewTicker(time.Duration(period) * time.Second)
+}
+
+// SetPayload will define a payload to carry with multicast messages. Currently setting the
+// payload once StartMulticast has been called is not supported, so this function must be called
+// always before StartMulticast()
+func (mc *Multicast) SetPayload(payload string) {
+	mc.payload = payload
+}
+
 // StartMulticast initiates advertising ourselves through multicasting
 func (mc *Multicast) StartMulticast() {
 	// Periodically announce ourselves to the network
@@ -120,11 +135,10 @@ func (mc *Multicast) ListenPeers() {
 // users of this library when the program exits or the discovery service is disabled by
 // the end user
 func (mc *Multicast) LeaveMulticast() {
-	// Stop sending hello
-	if mc.helloTicker != nil {
-		mc.helloTicker.Stop()
-	}
-	msg, e := makeByeMessage(mc.Payload).serialize()
+	// Stop sending 'hello'
+	mc.helloTicker.Stop()
+
+	msg, e := makeByeMessage(mc.payload).serialize()
 	if e != nil {
 		log.Error(e)
 	}
@@ -158,8 +172,7 @@ func (mc *Multicast) read(b []byte) (int, *net.UDPAddr, error) {
 // sendHellos returns an error if failing to set up the message, otherwise it handles its own
 // errors (i.e. it will keep sending hellos after a failure).
 func (mc *Multicast) sendHellos() error {
-	mc.helloTicker = time.NewTicker(time.Duration(mc.Period) * time.Second)
-	msg, e := makeHelloMessage(mc.Payload).serialize()
+	msg, e := makeHelloMessage(mc.payload).serialize()
 	if e != nil {
 		return e
 	}
@@ -191,7 +204,7 @@ func (mc *Multicast) listenPeers() error {
 			}
 
 			// Set a deadline to avoid blocking on a read forever
-			e := mc.conn.SetReadDeadline(time.Now().Add(time.Duration(mc.Period) * time.Second))
+			e := mc.conn.SetReadDeadline(time.Now().Add(time.Duration(mc.period) * time.Second))
 			if e != nil {
 				log.Error(e)
 			}
diff --git a/src/github.com/getlantern/multicast/multicast_test.go b/src/github.com/getlantern/multicast/multicast_test.go
@@ -101,8 +101,8 @@ func TestMulticastMessages(t *testing.T) {
 		t.Fatal("Unable to set socket for multicast looping")
 	}
 
-	mc1.Period = 1
-	mc1.Payload = "testHello"
+	mc1.SetPayload("testHello")
+	mc1.SetPeriod(1)
 	mc1.StartMulticast()
 	mc1.ListenPeers()
 
@@ -144,7 +144,7 @@ func TestMulticastAnnouncing(t *testing.T) {
 		t.Fatal("Unable to set socket for multicast looping")
 	}
 
-	mc1.Period = 1
+	mc1.SetPeriod(1)
 	go func() {
 		if e := mc1.sendHellos(); e != nil {
 			log.Fatal("Error sending hellos")

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,6 @@ import (`
`12`	`12`
`13`	`13`	`const (`
`14`	`14`	messageType = `LocalDiscovery`
`15`		`- updatePeriod = 10`
`16`	`15`	`)`
`17`	`16`
`18`	`17`	`var (`
`@@ -52,7 +51,7 @@ func Start(advertise bool, portToAdvertise string) {`
`52`	`51`	`mc = multicast.JoinMulticast(addOrRemoveCb, addOrRemoveCb)`
`53`	`52`
`54`	`53`	`if advertise {`
`55`		`- mc.Payload = portToAdvertise`
	`54`	`+ mc.SetPayload(portToAdvertise)`
`56`	`55`	`mc.StartMulticast()`
`57`	`56`	`}`
`58`	`57`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ func main() {`
`25`	`25`	`log.Fatal("Error setting up socket for multicast loop")`
`26`	`26`	`}`
`27`	`27`	`*/`
`28`		`- mc.Period = 1`
	`28`	`+ mc.SetPeriod(1)`
`29`	`29`	`mc.StartMulticast()`
`30`	`30`	`mc.ListenPeers()`
`31`	`31`