Revisit waitgroup (#241)

* Fix Add to waitgroup out of goroutines calling wg.Done()
* Pass waitgroup to other loop functions

cmd/gluetun/main.go

@@ -194,6 +194,7 @@ func _main(background context.Context, args []string) int { //nolint:gocognit,go
 	portForward := openvpnLooper.PortForward
 	getOpenvpnSettings := openvpnLooper.GetSettings
 	getPortForwarded := openvpnLooper.GetPortForwarded
+	wg.Add(1)
 	// wait for restartOpenvpn
 	go openvpnLooper.Run(ctx, wg)
 
@@ -205,22 +206,27 @@ func _main(background context.Context, args []string) int { //nolint:gocognit,go
 
 	unboundLooper := dns.NewLooper(dnsConf, allSettings.DNS, logger, streamMerger, uid, gid)
 	restartUnbound := unboundLooper.Restart
-	// wait for restartUnbound
+	wg.Add(1)
+	// wait for restartUnbound or its ticker launched with RunRestartTicker
 	go unboundLooper.Run(ctx, wg)
 
 	publicIPLooper := publicip.NewLooper(client, logger, fileManager, allSettings.System.IPStatusFilepath, allSettings.PublicIPPeriod, uid, gid)
 	restartPublicIP := publicIPLooper.Restart
 	setPublicIPPeriod := publicIPLooper.SetPeriod
-	go publicIPLooper.Run(ctx)
+	wg.Add(1)
-	go publicIPLooper.RunRestartTicker(ctx)
+	go publicIPLooper.Run(ctx, wg)
+	wg.Add(1)
+	go publicIPLooper.RunRestartTicker(ctx, wg)
 	setPublicIPPeriod(allSettings.PublicIPPeriod) // call after RunRestartTicker
 
 	tinyproxyLooper := tinyproxy.NewLooper(tinyProxyConf, firewallConf, allSettings.TinyProxy, logger, streamMerger, uid, gid, defaultInterface)
 	restartTinyproxy := tinyproxyLooper.Restart
+	wg.Add(1)
 	go tinyproxyLooper.Run(ctx, wg)
 
 	shadowsocksLooper := shadowsocks.NewLooper(firewallConf, allSettings.ShadowSocks, logger, defaultInterface)
 	restartShadowsocks := shadowsocksLooper.Restart
+	wg.Add(1)
 	go shadowsocksLooper.Run(ctx, wg)
 
 	if allSettings.TinyProxy.Enabled {
@@ -241,19 +247,26 @@ func _main(background context.Context, args []string) int { //nolint:gocognit,go
 		}
 		logger.Info(message)
 	}
+	wg.Add(1)
 	go func() {
+		defer wg.Done()
+		tickerWg := &sync.WaitGroup{}
+		// for linters only
 		var restartTickerContext context.Context
 		var restartTickerCancel context.CancelFunc = func() {}
 		for {
 			select {
 			case <-ctx.Done():
-				restartTickerCancel()
+				restartTickerCancel() // for linters only
+				tickerWg.Wait()
 				return
 			case <-connectedCh: // blocks until openvpn is connected
-				restartTickerCancel()
+				restartTickerCancel() // stop previous restart tickers
+				tickerWg.Wait()
 				restartTickerContext, restartTickerCancel = context.WithCancel(ctx)
-				go unboundLooper.RunRestartTicker(restartTickerContext)
+				tickerWg.Add(2)
-				go updaterLooper.RunRestartTicker(ctx)
+				go unboundLooper.RunRestartTicker(restartTickerContext, tickerWg)
+				go updaterLooper.RunRestartTicker(restartTickerContext, tickerWg)
 				onConnected(allSettings, logger, routingConf, portForward, restartUnbound, restartPublicIP, versionInformation)
 			}
 		}
@@ -261,6 +274,7 @@ func _main(background context.Context, args []string) int { //nolint:gocognit,go
 
 	httpServer := server.New("0.0.0.0:8000", logger, restartOpenvpn, restartUnbound, updaterLooper.Restart,
 		getOpenvpnSettings, getPortForwarded)
+	wg.Add(1)
 	go httpServer.Run(ctx, wg)
 
 	// Start openvpn for the first time

internal/dns/loop.go

@@ -14,7 +14,7 @@ import (
 
 type Looper interface {
 	Run(ctx context.Context, wg *sync.WaitGroup)
-	RunRestartTicker(ctx context.Context)
+	RunRestartTicker(ctx context.Context, wg *sync.WaitGroup)
 	Restart()
 	Start()
 	Stop()
@@ -139,7 +139,6 @@ func (l *looper) waitForSubsequentStart(ctx context.Context, unboundCancel conte
 }
 
 func (l *looper) Run(ctx context.Context, wg *sync.WaitGroup) {
-	wg.Add(1)
 	defer wg.Done()
 	const fallback = false
 	l.useUnencryptedDNS(fallback)
@@ -282,7 +281,8 @@ func (l *looper) useUnencryptedDNS(fallback bool) {
 	l.logger.Error("no ipv4 DNS address found for providers %s", settings.Providers)
 }
 
-func (l *looper) RunRestartTicker(ctx context.Context) {
+func (l *looper) RunRestartTicker(ctx context.Context, wg *sync.WaitGroup) {
+	defer wg.Done()
 	ticker := time.NewTicker(time.Hour)
 	settings := l.GetSettings()
 	if settings.UpdatePeriod > 0 {

internal/openvpn/loop.go

@@ -98,7 +98,6 @@ func (l *looper) SetAllServers(allServers models.AllServers) {
 }
 
 func (l *looper) Run(ctx context.Context, wg *sync.WaitGroup) {
-	wg.Add(1)
 	defer wg.Done()
 	select {
 	case <-l.restart:

internal/publicip/loop.go

@@ -12,8 +12,8 @@ import (
 )
 
 type Looper interface {
-	Run(ctx context.Context)
+	Run(ctx context.Context, wg *sync.WaitGroup)
-	RunRestartTicker(ctx context.Context)
+	RunRestartTicker(ctx context.Context, wg *sync.WaitGroup)
 	Restart()
 	Stop()
 	GetPeriod() (period time.Duration)
@@ -74,7 +74,8 @@ func (l *looper) logAndWait(ctx context.Context, err error) {
 	<-ctx.Done()
 }
 
-func (l *looper) Run(ctx context.Context) {
+func (l *looper) Run(ctx context.Context, wg *sync.WaitGroup) {
+	defer wg.Done()
 	select {
 	case <-l.restart:
 	case <-ctx.Done():
@@ -124,7 +125,8 @@ func (l *looper) Run(ctx context.Context) {
 	}
 }
 
-func (l *looper) RunRestartTicker(ctx context.Context) {
+func (l *looper) RunRestartTicker(ctx context.Context, wg *sync.WaitGroup) {
+	defer wg.Done()
 	ticker := time.NewTicker(time.Hour)
 	period := l.GetPeriod()
 	if period > 0 {

internal/server/server.go

@@ -42,7 +42,6 @@ func New(address string, logger logging.Logger, restartOpenvpn, restartUnbound,
 }
 
 func (s *server) Run(ctx context.Context, wg *sync.WaitGroup) {
-	wg.Add(1)
 	server := http.Server{Addr: s.address, Handler: s.makeHandler()}
 	go func() {
 		defer wg.Done()

internal/shadowsocks/loop.go

@@ -82,7 +82,6 @@ func (l *looper) setEnabled(enabled bool) {
 }
 
 func (l *looper) Run(ctx context.Context, wg *sync.WaitGroup) {
-	wg.Add(1)
 	defer wg.Done()
 	waitForStart := true
 	for waitForStart {

internal/tinyproxy/loop.go

@@ -89,7 +89,6 @@ func (l *looper) Start()   { l.start <- struct{}{} }
 func (l *looper) Stop()    { l.stop <- struct{}{} }
 
 func (l *looper) Run(ctx context.Context, wg *sync.WaitGroup) {
-	wg.Add(1)
 	defer wg.Done()
 	waitForStart := true
 	for waitForStart {

internal/updater/loop.go

@@ -13,7 +13,7 @@ import (
 
 type Looper interface {
 	Run(ctx context.Context, wg *sync.WaitGroup)
-	RunRestartTicker(ctx context.Context)
+	RunRestartTicker(ctx context.Context, wg *sync.WaitGroup)
 	Restart()
 	Stop()
 	GetPeriod() (period time.Duration)
@@ -123,7 +123,8 @@ func (l *looper) Run(ctx context.Context, wg *sync.WaitGroup) {
 	}
 }
 
-func (l *looper) RunRestartTicker(ctx context.Context) {
+func (l *looper) RunRestartTicker(ctx context.Context, wg *sync.WaitGroup) {
+	defer wg.Done()
 	ticker := time.NewTicker(time.Hour)
 	period := l.GetPeriod()
 	if period > 0 {