Block rtxTimer.stop() until timer routine end

Fix data race during TestRtxTimer.

README.md

@@ -48,6 +48,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [Lukas Herman](https://github.com/lherman-cs)
 * [Luke Curley](https://github.com/kixelated) - *Performance*
 * [Aaron France](https://github.com/AeroNotix)
+* [Atsushi Watanabe](https://github.com/at-wat)
 
 ### License
 MIT License - see [LICENSE](LICENSE) for full text

rtx_timer.go

@@ -101,6 +101,7 @@ type rtxTimer struct {
 	observer   rtxTimerObserver
 	maxRetrans uint
 	stopFunc   stopTimerLoop
+	runningCh  <-chan struct{}
 	closed     bool
 	mutex      sync.RWMutex
 }
@@ -140,8 +141,12 @@ func (t *rtxTimer) start(rto float64) bool {
 	var nRtos uint
 
 	cancelCh := make(chan struct{})
+	runningCh := make(chan struct{})
 
 	go func() {
+		defer func() {
+			close(runningCh)
+		}()
 		canceling := false
 
 		for !canceling {
@@ -154,7 +159,7 @@ func (t *rtxTimer) start(rto float64) bool {
 				if t.maxRetrans == 0 || nRtos <= t.maxRetrans {
 					t.observer.onRetransmissionTimeout(t.id, nRtos)
 				} else {
-					t.stop()
+					t.stopInternal()
 					t.observer.onRetransmissionFailure(t.id)
 				}
 			case <-cancelCh:
@@ -164,21 +169,30 @@ func (t *rtxTimer) start(rto float64) bool {
 		}
 	}()
 
+	t.runningCh = runningCh
 	t.stopFunc = func() {
 		close(cancelCh)
 	}
 
 	return true
 }
 
+// stopInternal stops the timer. t.mutex must be locked before calling this.
+func (t *rtxTimer) stopInternal() {
+	if t.stopFunc != nil {
+		t.stopFunc()
+		t.stopFunc = nil
+	}
+}
+
 // stop stops the timer.
 func (t *rtxTimer) stop() {
 	t.mutex.Lock()
 	defer t.mutex.Unlock()
 
-	if t.stopFunc != nil {
-		t.stopFunc()
-		t.stopFunc = nil
+	t.stopInternal()
+	if t.runningCh != nil {
+		<-t.runningCh // wait until timer routine stops
 	}
 }
 
@@ -188,9 +202,9 @@ func (t *rtxTimer) close() {
 	t.mutex.Lock()
 	defer t.mutex.Unlock()
 
-	if t.stopFunc != nil {
-		t.stopFunc()
-		t.stopFunc = nil
+	t.stopInternal()
+	if t.runningCh != nil {
+		<-t.runningCh // wait until timer routine stops
 	}
 
 	t.closed = true