From 0f5c55307046604300880d1f461a629e7715ebba Mon Sep 17 00:00:00 2001
From: Atsushi Watanabe <atsushi.w@ieee.org>
Date: Sun, 28 Jul 2019 15:27:51 +0900
Subject: [PATCH] Fix Ticker leak

time.Ticker leaks if not Stop()-ed after use.
Close connectivity interval ticker on Agent.Close() to fix leak.
---
 agent.go      | 13 ++++++++-----
 agent_test.go |  5 +++++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/agent.go b/agent.go
index 3b1cf20..a36423e 100644
--- a/agent.go
+++ b/agent.go
@@ -79,8 +79,8 @@ type Agent struct {
 	onConnected     chan struct{}
 	onConnectedOnce sync.Once
 
-	connectivityChan <-chan time.Time
-	// force candidate to be contacted immediately (instead of waiting for connectivityChan)
+	connectivityTicker *time.Ticker
+	// force candidate to be contacted immediately (instead of waiting for connectivityTicker)
 	forceCandidateContact chan bool
 
 	trickle         bool
@@ -462,8 +462,7 @@ func (a *Agent) startConnectivityChecks(isControlling bool, remoteUfrag, remoteP
 
 		// TODO this should be dynamic, and grow when the connection is stable
 		agent.forceCandidateContact <- true
-		t := time.NewTicker(a.taskLoopInterval)
-		agent.connectivityChan = t.C
+		agent.connectivityTicker = time.NewTicker(a.taskLoopInterval)
 	})
 }
 
@@ -586,7 +585,7 @@ func (a *Agent) taskLoop() {
 			select {
 			case <-a.forceCandidateContact:
 				a.selector.ContactCandidates()
-			case <-a.connectivityChan:
+			case <-a.connectivityTicker.C:
 				a.selector.ContactCandidates()
 			case t := <-a.taskChan:
 				// Run the task
@@ -787,6 +786,10 @@ func (a *Agent) Close() error {
 			a.log.Warnf("failed to close buffer: %v", err)
 		}
 
+		if a.connectivityTicker != nil {
+			a.connectivityTicker.Stop()
+		}
+
 		a.closeMulticastConn()
 	})
 	if err != nil {
diff --git a/agent_test.go b/agent_test.go
index 20d926e..b5c3736 100644
--- a/agent_test.go
+++ b/agent_test.go
@@ -222,6 +222,7 @@ func TestHandlePeerReflexive(t *testing.T) {
 		var config AgentConfig
 		runAgentTest(t, &config, func(a *Agent) {
 			a.selector = &controllingSelector{agent: a, log: a.log}
+			a.connectivityTicker = time.NewTicker(a.taskLoopInterval)
 
 			hostConfig := CandidateHostConfig{
 				Network:   "udp",
@@ -287,6 +288,7 @@ func TestHandlePeerReflexive(t *testing.T) {
 		var config AgentConfig
 		runAgentTest(t, &config, func(a *Agent) {
 			a.selector = &controllingSelector{agent: a, log: a.log}
+			a.connectivityTicker = time.NewTicker(a.taskLoopInterval)
 
 			hostConfig := CandidateHostConfig{
 				Network:   "tcp",
@@ -318,6 +320,7 @@ func TestHandlePeerReflexive(t *testing.T) {
 		var config AgentConfig
 		runAgentTest(t, &config, func(a *Agent) {
 			a.selector = &controllingSelector{agent: a, log: a.log}
+			a.connectivityTicker = time.NewTicker(a.taskLoopInterval)
 			tID := [stun.TransactionIDSize]byte{}
 			copy(tID[:], []byte("ABC"))
 			a.pendingBindingRequests = []bindingRequest{
@@ -470,6 +473,7 @@ func TestInboundValidity(t *testing.T) {
 
 		err = a.run(func(a *Agent) {
 			a.selector = &controllingSelector{agent: a, log: a.log}
+			a.connectivityTicker = time.NewTicker(a.taskLoopInterval)
 			a.handleInbound(buildMsg(stun.ClassRequest, a.localUfrag+":"+a.remoteUfrag, a.localPwd), local, remote)
 			if len(a.remoteCandidates) != 1 {
 				t.Fatal("Binding with valid values was unable to create prflx candidate")
@@ -485,6 +489,7 @@ func TestInboundValidity(t *testing.T) {
 		var config AgentConfig
 		runAgentTest(t, &config, func(a *Agent) {
 			a.selector = &controllingSelector{agent: a, log: a.log}
+			a.connectivityTicker = time.NewTicker(a.taskLoopInterval)
 			msg, err := stun.Build(stun.BindingRequest, stun.TransactionID,
 				stun.NewUsername(a.localUfrag+":"+a.remoteUfrag),
 				stun.NewShortTermIntegrity(a.localPwd),