core: eliminate potential race conditions from Events and Every functions

Signed-off-by: deadprogram <ron@hybridgroup.com>

eventer.go

@@ -1,5 +1,7 @@
 package gobot
 
+import "sync"
+
 type eventChannel chan *Event
 
 type eventer struct {
@@ -11,6 +13,9 @@ type eventer struct {
 
 	// map of out channels used by subscribers
 	outs map[eventChannel]eventChannel
+
+	// mutex to protect the eventChannel map
+	eventsMutex sync.Mutex
 }
 
 // Eventer is the interface which describes how a Driver or Adaptor
@@ -58,9 +63,11 @@ func NewEventer() Eventer {
 		for {
 			select {
 			case evt := <-evtr.in:
+				evtr.eventsMutex.Lock()
 				for _, out := range evtr.outs {
 					out <- evt
 				}
+				evtr.eventsMutex.Unlock()
 			}
 		}
 	}()
@@ -97,6 +104,8 @@ func (e *eventer) Publish(name string, data interface{}) {
 
 // Subscribe to any events from this eventer
 func (e *eventer) Subscribe() eventChannel {
+	e.eventsMutex.Lock()
+	defer e.eventsMutex.Unlock()
 	out := make(eventChannel)
 	e.outs[out] = out
 	return out
@@ -104,6 +113,8 @@ func (e *eventer) Subscribe() eventChannel {
 
 // Unsubscribe from the event channel
 func (e *eventer) Unsubscribe(events eventChannel) {
+	e.eventsMutex.Lock()
+	defer e.eventsMutex.Unlock()
 	delete(e.outs, events)
 }
 

examples/every_done.go

@@ -11,12 +11,12 @@ func main() {
 	robot := gobot.NewRobot(
 		"hello",
 		func() {
-			done := gobot.Every(500*time.Millisecond, func() {
+			done := gobot.Every(750*time.Millisecond, func() {
 				fmt.Println("Greetings human")
 			})
 
 			gobot.After(5*time.Second, func() {
-				done <- true
+				done.Stop()
 				fmt.Println("We're done here")
 			})
 		},

utils.go

@@ -23,27 +23,23 @@ var eventError = func(e *Event) (err error) {
 	return
 }
 
-// Every triggers f every t time until the end of days, or when a
-// bool value is sent to the channel returned by the Every function.
+// Every triggers f every t time.Duration until the end of days, or when a Stop()
+// is called on the Ticker that is returned by the Every function.
 // It does not wait for the previous execution of f to finish before
 // it fires the next f.
-func Every(t time.Duration, f func()) chan bool {
-	done := make(chan bool)
-	c := time.Tick(t)
+func Every(t time.Duration, f func()) *time.Ticker {
+	ticker := time.NewTicker(t)
 
 	go func() {
 		for {
 			select {
-			case <-done:
-				return
-			default:
-				<-c
-				go f()
+			case <-ticker.C:
+				f()
 			}
 		}
 	}()
 
-	return done
+	return ticker
 }
 
 // After triggers f after t duration.

utils_test.go

@@ -23,25 +23,42 @@ func TestEvery(t *testing.T) {
 	}
 }
 
-func TestEveryWhenDone(t *testing.T) {
-	i := 0
-	done := Every(20*time.Millisecond, func() {
-		i++
+func TestEveryWhenStopped(t *testing.T) {
+	sem := make(chan bool)
+
+	done := Every(50*time.Millisecond, func() {
+		sem <- true
 	})
-	time.Sleep(10 * time.Millisecond)
-	done <- true
-	time.Sleep(50 * time.Millisecond)
-	if i > 1 {
-		t.Error("Test should have stopped after 20ms")
+
+	select {
+	case <-sem:
+		done.Stop()
+	case <-time.After(60 * time.Millisecond):
+		t.Errorf("Every was not called")
+	}
+
+	select {
+	case <-time.After(60 * time.Millisecond):
+	case <-sem:
+		t.Error("Every should have stopped")
 	}
 }
 
 func TestAfter(t *testing.T) {
 	i := 0
+	sem := make(chan bool)
+
 	After(1*time.Millisecond, func() {
 		i++
+		sem <- true
 	})
-	time.Sleep(2 * time.Millisecond)
+
+	select {
+	case <-sem:
+	case <-time.After(10 * time.Millisecond):
+		t.Errorf("After was not called")
+	}
+
 	gobottest.Assert(t, i, 1)
 }