[BUGFIX] Fix race condition in freeport (#7567)

This removes a race condition in reset since pendingPorts can be set to nil in reset()

If ticker is hit at wrong time, it would crash the unit test.

We ensure in reset to avoid this race condition by cancelling the goroutine using
killTicker chan.

We also properly clean up eveything, so garbage collector can work as expected.

To reproduce existing bug:
`while go test -timeout 30s github.com/hashicorp/consul/sdk/freeport -run '^(Test.*)$'; do go clean -testcache; done`

Will crash after a few 10s runs on my machine.

Error could be seen in unit tests sometimes:

[INFO] freeport: resetting the freeport package state
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x1125536]

goroutine 25 [running]:
container/list.(*List).Len(...)
	/usr/local/Cellar/go/1.14/libexec/src/container/list/list.go:66
github.com/hashicorp/consul/sdk/freeport.checkFreedPortsOnce()
	/Users/p.souchay/go/src/github.com/hashicorp/consul/sdk/freeport/freeport.go:157 +0x86
github.com/hashicorp/consul/sdk/freeport.checkFreedPorts()
	/Users/p.souchay/go/src/github.com/hashicorp/consul/sdk/freeport/freeport.go:147 +0x71
created by github.com/hashicorp/consul/sdk/freeport.initialize
	/Users/p.souchay/go/src/github.com/hashicorp/consul/sdk/freeport/freeport.go:113 +0x2cf
FAIL	github.com/hashicorp/consul/sdk/freeport	1.607s

sdk/freeport/freeport.go

@@ -66,6 +66,14 @@ var (
 
 	// total is the total number of available ports in the block for use.
 	total int
+
+	// stopCh is used to signal to background goroutines to terminate. Only
+	// really exists for the safety of reset() during unit tests.
+	stopCh chan struct{}
+
+	// stopWg is used to keep track of background goroutines that are still
+	// alive. Only really exists for the safety of reset() during unit tests.
+	stopWg sync.WaitGroup
 )
 
 // initialize is used to initialize freeport.
@@ -108,16 +116,36 @@ func initialize() {
 	}
 	total = freePorts.Len()
 
-	go checkFreedPorts()
+	stopWg.Add(1)
+	stopCh = make(chan struct{})
+	// Note: we pass this param explicitly to the goroutine so that we can
+	// freely recreate the underlying stop channel during reset() after closing
+	// the original.
+	go checkFreedPorts(stopCh)
+}
+
+func shutdownGoroutine() {
+	mu.Lock()
+	if stopCh == nil {
+		mu.Unlock()
+		return
+	}
+
+	close(stopCh)
+	stopCh = nil
+	mu.Unlock()
+
+	stopWg.Wait()
 }
 
 // reset will reverse the setup from initialize() and then redo it (for tests)
 func reset() {
+	logf("INFO", "resetting the freeport package state")
+	shutdownGoroutine()
+
 	mu.Lock()
 	defer mu.Unlock()
 
-	logf("INFO", "resetting the freeport package state")
-
 	effectiveMaxBlocks = 0
 	firstPort = 0
 	if lockLn != nil {
@@ -132,11 +160,18 @@ func reset() {
 	total = 0
 }
 
-func checkFreedPorts() {
+func checkFreedPorts(stopCh <-chan struct{}) {
+	defer stopWg.Done()
+
 	ticker := time.NewTicker(250 * time.Millisecond)
 	for {
-		<-ticker.C
-		checkFreedPortsOnce()
+		select {
+		case <-stopCh:
+			logf("INFO", "Closing checkFreedPorts()")
+			return
+		case <-ticker.C:
+			checkFreedPortsOnce()
+		}
 	}
 }