test: use goleak to autodetect leaks

test that redistest does not leak goroutines
fix: found goroutine leaks
fix: wrap close in sync.once.Do
fix: shallow copy can not copy by deref ptr in case it contains sync.Once
refactor: *sync-Once to sync.Once as shown in stdlib Go example

fix as commented: cleanup nil check
fix as commented: remove t.closed in httpclient
test: add test that NewClient will never return nil
fix as commented: remove time.Sleep
fix as commented: use embedding to get rid of delegations

Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

dataclients/kubernetes/main_test.go

@@ -0,0 +1,11 @@
+package kubernetes_test
+
+import (
+	"testing"
+
+	"go.uber.org/goleak"
+)
+
+func TestMain(m *testing.M) {
+	goleak.VerifyTestMain(m)
+}

eskip/eskip_test.go

@@ -323,13 +323,15 @@ func TestParseFilters(t *testing.T) {
 		[]*Filter{{Name: "filter1", Args: []interface{}{3.14}}, {Name: "filter2", Args: []interface{}{"key", float64(42)}}},
 		false,
 	}} {
-		fs, err := ParseFilters(ti.expression)
-		if err == nil && ti.err || err != nil && !ti.err {
-			t.Error(ti.msg, "failure case", err, ti.err)
-			return
-		}
+		t.Run(ti.msg, func(t *testing.T) {
+			fs, err := ParseFilters(ti.expression)
+			if err == nil && ti.err || err != nil && !ti.err {
+				t.Error(ti.msg, "failure case", err, ti.err)
+				return
+			}
 
-		checkFilters(t, ti.msg, fs, ti.check)
+			checkFilters(t, ti.msg, fs, ti.check)
+		})
 	}
 }
 
@@ -634,7 +636,7 @@ func TestClonePreProcessor(t *testing.T) {
 				repl: "HostAny($1)",
 			},
 			routes: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>;`,
-			want: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>; 
+			want: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>;
 			clone_r0: HostAny("www[.]example[.]org") -> status(201) -> <shunt>;`,
 		},
 		{
@@ -653,7 +655,7 @@ func TestClonePreProcessor(t *testing.T) {
 				repl: "ClientIP($1)",
 			},
 			routes: `r1: Source("1.2.3.4/26") -> status(201) -> <shunt>;`,
-			want: `r1: Source("1.2.3.4/26") -> status(201) -> <shunt>; 
+			want: `r1: Source("1.2.3.4/26") -> status(201) -> <shunt>;
 			clone_r1: ClientIP("1.2.3.4/26") -> status(201) -> <shunt>;`,
 		},
 		{
@@ -662,11 +664,11 @@ func TestClonePreProcessor(t *testing.T) {
 				reg:  regexp.MustCompile("Source[(](.*)[)]"),
 				repl: "ClientIP($1)",
 			},
-			routes: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>; 
+			routes: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>;
 			r1: Source("1.2.3.4/26") -> status(201) -> <shunt>;`,
 
-			want: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>; 
-			r1: Source("1.2.3.4/26") -> status(201) -> <shunt>; 
+			want: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>;
+			r1: Source("1.2.3.4/26") -> status(201) -> <shunt>;
 			clone_r1: ClientIP("1.2.3.4/26") -> status(201) -> <shunt>;`,
 		},
 		{
@@ -676,7 +678,7 @@ func TestClonePreProcessor(t *testing.T) {
 				repl: "ClientIP($1)",
 			},
 			routes: `rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>;`,
-			want: `rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>; 
+			want: `rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>;
 			clone_rn: ClientIP("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>;`,
 		},
 		{
@@ -687,8 +689,8 @@ func TestClonePreProcessor(t *testing.T) {
 			},
 			routes: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>;
 			rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>;`,
-			want: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>; 
-			rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>; 
+			want: `r0: Host("www[.]example[.]org") -> status(201) -> <shunt>;
+			rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>;
 			clone_rn: ClientIP("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> <shunt>;`,
 		},
 		{
@@ -698,7 +700,7 @@ func TestClonePreProcessor(t *testing.T) {
 				repl: "normalRequestLatency($1)",
 			},
 			routes: `r1_filter: Source("1.2.3.4/26") -> uniformRequestLatency("100ms", "10ms") -> status(201) -> <shunt>;`,
-			want: `r1_filter: Source("1.2.3.4/26") -> uniformRequestLatency("100ms", "10ms") -> status(201) -> <shunt>; 
+			want: `r1_filter: Source("1.2.3.4/26") -> uniformRequestLatency("100ms", "10ms") -> status(201) -> <shunt>;
 			clone_r1_filter: Source("1.2.3.4/26") -> normalRequestLatency("100ms", "10ms") -> status(201) -> <shunt>;`,
 		}} {
 		t.Run(tt.name, func(t *testing.T) {

eskip/main_test.go

@@ -0,0 +1,11 @@
+package eskip_test
+
+import (
+	"testing"
+
+	"go.uber.org/goleak"
+)
+
+func TestMain(m *testing.M) {
+	goleak.VerifyTestMain(m)
+}

eskipfile/remote.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"os"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/zalando/skipper/eskip"
@@ -15,6 +16,7 @@ import (
 )
 
 type remoteEskipFile struct {
+	once            sync.Once
 	preloaded       bool
 	remotePath      string
 	localPath       string
@@ -55,6 +57,7 @@ func RemoteWatch(o *RemoteWatchOptions) (routing.DataClient, error) {
 	}
 
 	dataClient := &remoteEskipFile{
+		once:       sync.Once{},
 		remotePath: o.RemoteFile,
 		localPath:  tempFilename.Name(),
 		threshold:  o.Threshold,
@@ -135,6 +138,15 @@ func (client *remoteEskipFile) LoadUpdate() ([]*eskip.Route, []string, error) {
 	return newRoutes, deletedRoutes, err
 }
 
+func (client *remoteEskipFile) Close() {
+	if client != nil {
+		client.once.Do(func() {
+			client.http.Close()
+			client.eskipFileClient.Close()
+		})
+	}
+}
+
 func isFileRemote(remotePath string) bool {
 	return strings.HasPrefix(remotePath, "http://") || strings.HasPrefix(remotePath, "https://")
 }

eskipfile/remote_test.go

@@ -91,6 +91,12 @@ func TestLoadAll(t *testing.T) {
 		t.Run(test.title, func(t *testing.T) {
 			options := &RemoteWatchOptions{RemoteFile: s.URL, Threshold: 10, Verbose: true, FailOnStartup: true}
 			client, err := RemoteWatch(options)
+			defer func() {
+				c, ok := client.(*remoteEskipFile)
+				if ok {
+					c.Close()
+				}
+			}()
 			if err == nil && test.fail {
 				t.Error("failed to fail")
 				return

eskipfile/watch.go

@@ -3,6 +3,7 @@ package eskipfile
 import (
 	"os"
 	"reflect"
+	"sync"
 
 	"github.com/zalando/skipper/eskip"
 )
@@ -21,6 +22,7 @@ type WatchClient struct {
 	getAll     chan (chan<- watchResponse)
 	getUpdates chan (chan<- watchResponse)
 	quit       chan struct{}
+	once       sync.Once
 }
 
 // Watch creates a route configuration client with file watching. Watch doesn't follow file system nodes, it
@@ -31,6 +33,7 @@ func Watch(name string) *WatchClient {
 		getAll:     make(chan (chan<- watchResponse)),
 		getUpdates: make(chan (chan<- watchResponse)),
 		quit:       make(chan struct{}),
+		once:       sync.Once{},
 	}
 
 	go c.watch()
@@ -157,5 +160,7 @@ func (c *WatchClient) LoadUpdate() ([]*eskip.Route, []string, error) {
 
 // Close stops watching the configured file and providing updates.
 func (c *WatchClient) Close() {
-	close(c.quit)
+	c.once.Do(func() {
+		close(c.quit)
+	})
 }

filters/serve/serve_test.go

@@ -121,6 +121,7 @@ func TestServe(t *testing.T) {
 	if err != nil || string(b) != strings.Join(parts, "") {
 		t.Error("failed to serve body")
 	}
+	ctx.Response().Body.Close()
 }
 
 func TestStreamBody(t *testing.T) {

go.mod

@@ -33,7 +33,7 @@ require (
 	github.com/sarslanhan/cronmask v0.0.0-20190709075623-766eca24d011
 	github.com/sirupsen/logrus v1.8.1
 	github.com/sony/gobreaker v0.5.0
-	github.com/stretchr/testify v1.7.0
+	github.com/stretchr/testify v1.8.0
 	github.com/szuecs/rate-limit-buffer v0.7.1
 	github.com/testcontainers/testcontainers-go v0.12.0
 	github.com/tidwall/gjson v1.12.1
@@ -43,6 +43,7 @@ require (
 	github.com/yookoala/gofast v0.6.0
 	github.com/yuin/gopher-lua v0.0.0-20210529063254-f4c35e4016d9
 	go.uber.org/atomic v1.9.0
+	go.uber.org/goleak v1.2.0
 	golang.org/x/crypto v0.0.0-20220817201139-bc19a97f63c8
 	golang.org/x/net v0.0.0-20220909164309-bea034e7d591
 	golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8

go.sum

@@ -803,6 +803,7 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
 github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/testify v0.0.0-20161117074351-18a02ba4a312/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v0.0.0-20180303142811-b89eecf5ca5d/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
@@ -812,6 +813,9 @@ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/syndtr/gocapability v0.0.0-20170704070218-db04d3cc01c8/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww=
 github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww=
 github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww=
@@ -891,6 +895,8 @@ go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE=
 go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
 go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ=
+go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk=
+go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo=
 go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
 go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
 golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=

net/httpclient.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptrace"
 	"net/url"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/opentracing/opentracing-go"
@@ -24,6 +25,7 @@ const (
 // opentracing to the wrapped http.Client with the same interface as
 // http.Client from the stdlib.
 type Client struct {
+	once   sync.Once
 	client http.Client
 	tr     *Transport
 	log    logging.Logger
@@ -58,6 +60,7 @@ func NewClient(o Options) *Client {
 	}
 
 	c := &Client{
+		once: sync.Once{},
 		client: http.Client{
 			Transport: tr,
 		},
@@ -70,10 +73,14 @@ func NewClient(o Options) *Client {
 }
 
 func (c *Client) Close() {
-	c.tr.Close()
-	if c.sr != nil {
-		c.sr.Close()
-	}
+	c.once.Do(func() {
+		if c.tr != nil {
+			c.tr.Close()
+		}
+		if c.sr != nil {
+			c.sr.Close()
+		}
+	})
 }
 
 func (c *Client) Head(url string) (*http.Response, error) {
@@ -202,8 +209,8 @@ type Options struct {
 // Transport wraps an http.Transport and adds support for tracing and
 // bearerToken injection.
 type Transport struct {
+	once          sync.Once
 	quit          chan struct{}
-	closed        bool
 	tr            *http.Transport
 	tracer        opentracing.Tracer
 	spanName      string
@@ -265,6 +272,7 @@ func NewTransport(options Options) *Transport {
 	}
 
 	t := &Transport{
+		once:   sync.Once{},
 		quit:   make(chan struct{}),
 		tr:     htransport,
 		tracer: options.Tracer,
@@ -320,15 +328,21 @@ func WithBearerToken(t *Transport, bearerToken string) *Transport {
 }
 
 func (t *Transport) shallowCopy() *Transport {
-	tt := *t
-	return &tt
+	return &Transport{
+		once:          sync.Once{},
+		quit:          t.quit,
+		tr:            t.tr,
+		tracer:        t.tracer,
+		spanName:      t.spanName,
+		componentName: t.componentName,
+		bearerToken:   t.bearerToken,
+	}
 }
 
 func (t *Transport) Close() {
-	if !t.closed {
-		t.closed = true
+	t.once.Do(func() {
 		close(t.quit)
-	}
+	})
 }
 
 func (t *Transport) CloseIdleConnections() {

net/httpclient_test.go

@@ -36,6 +36,7 @@ func TestClient(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to get a tracer: %v", err)
 	}
+	defer tracer.Close()
 
 	for _, tt := range []struct {
 		name      string
@@ -157,6 +158,9 @@ func TestClient(t *testing.T) {
 			}
 
 			cli := NewClient(tt.options)
+			if cli == nil {
+				t.Fatal("NewClient returned nil")
+			}
 			defer cli.Close()
 
 			u := "http://" + s.Listener.Addr().String() + "/"
@@ -201,6 +205,7 @@ func TestTransport(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to get a tracer: %v", err)
 	}
+	defer tracer.Close()
 
 	for _, tt := range []struct {
 		name        string

net/main_test.go

@@ -0,0 +1,11 @@
+package net_test
+
+import (
+	"testing"
+
+	"go.uber.org/goleak"
+)
+
+func TestMain(m *testing.M) {
+	goleak.VerifyTestMain(m)
+}

net/redisclient.go

@@ -287,6 +287,7 @@ func (r *RedisRingClient) startUpdater(ctx context.Context) {
 	}
 
 	r.log.Infof("Start goroutine to update redis instances every %s", r.options.UpdateInterval)
+	defer r.log.Info("Stopped goroutine to update redis")
 
 	for {
 		select {
@@ -353,6 +354,7 @@ func (r *RedisRingClient) Close() {
 	r.once.Do(func() {
 		r.closed = true
 		close(r.quit)
+		r.ring.Close()
 	})
 }