From 7e50c9582303c568194afe95d729df178b263ff3 Mon Sep 17 00:00:00 2001
From: "Quentin McGaw (desktop)" <quentin.mcgaw@gmail.com>
Date: Fri, 16 Jul 2021 21:21:09 +0000
Subject: [PATCH] Maint: minor DNS loop fixes and changes

---
 internal/dns/loop.go  | 24 ++++++++++--------------
 internal/dns/state.go |  2 +-
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/internal/dns/loop.go b/internal/dns/loop.go
index 8f6a6a8e..c880e5b1 100644
--- a/internal/dns/loop.go
+++ b/internal/dns/loop.go
@@ -36,6 +36,7 @@ type looper struct {
 	blockBuilder blacklist.Builder
 	client       *http.Client
 	logger       logging.Logger
+	userTrigger  bool
 	start        <-chan struct{}
 	running      chan<- models.LoopStatus
 	stop         <-chan struct{}
@@ -65,6 +66,7 @@ func NewLooper(conf unbound.Configurator, settings configuration.DNS, client *ht
 		blockBuilder: blacklist.NewBuilder(client),
 		client:       client,
 		logger:       logger,
+		userTrigger:  true,
 		start:        start,
 		running:      running,
 		stop:         stop,
@@ -93,9 +95,9 @@ func (l *looper) logAndWait(ctx context.Context, err error) {
 	}
 }
 
-func (l *looper) signalOrSetStatus(userTriggered *bool, status models.LoopStatus) {
-	if *userTriggered {
-		*userTriggered = false
+func (l *looper) signalOrSetStatus(status models.LoopStatus) {
+	if l.userTrigger {
+		l.userTrigger = false
 		select {
 		case l.running <- status:
 		default: // receiver droppped out - avoid deadlock on events routing when shutting down
@@ -118,8 +120,6 @@ func (l *looper) Run(ctx context.Context, done chan<- struct{}) {
 		return
 	}
 
-	userTriggered := true
-
 	for ctx.Err() == nil {
 		// Upper scope variables for Unbound only
 		// Their values are to be used if DOT=off
@@ -133,11 +133,11 @@ func (l *looper) Run(ctx context.Context, done chan<- struct{}) {
 			if err == nil {
 				l.backoffTime = defaultBackoffTime
 				l.logger.Info("ready")
-				l.signalOrSetStatus(&userTriggered, constants.Running)
+				l.signalOrSetStatus(constants.Running)
 				break
 			}
 
-			l.signalOrSetStatus(&userTriggered, constants.Crashed)
+			l.signalOrSetStatus(constants.Crashed)
 
 			if ctx.Err() != nil {
 				return
@@ -155,7 +155,7 @@ func (l *looper) Run(ctx context.Context, done chan<- struct{}) {
 			l.useUnencryptedDNS(fallback)
 		}
 
-		userTriggered = false
+		l.userTrigger = false
 
 		stayHere := true
 		for stayHere {
@@ -167,7 +167,7 @@ func (l *looper) Run(ctx context.Context, done chan<- struct{}) {
 				closeStreams()
 				return
 			case <-l.stop:
-				userTriggered = true
+				l.userTrigger = true
 				l.logger.Info("stopping")
 				const fallback = false
 				l.useUnencryptedDNS(fallback)
@@ -178,23 +178,19 @@ func (l *looper) Run(ctx context.Context, done chan<- struct{}) {
 				closeStreams()
 				l.stopped <- struct{}{}
 			case <-l.start:
-				userTriggered = true
+				l.userTrigger = true
 				l.logger.Info("starting")
 				stayHere = false
 			case err := <-waitError: // unexpected error
 				close(waitError)
 				closeStreams()
 
-				l.state.Lock() // prevent SetStatus from running in parallel
-
 				unboundCancel()
 				l.state.SetStatus(constants.Crashed)
 				const fallback = true
 				l.useUnencryptedDNS(fallback)
 				l.logAndWait(ctx, err)
 				stayHere = false
-
-				l.state.Unlock()
 			}
 		}
 	}
diff --git a/internal/dns/state.go b/internal/dns/state.go
index 4d7b7774..9b214905 100644
--- a/internal/dns/state.go
+++ b/internal/dns/state.go
@@ -120,7 +120,7 @@ func (s *state) ApplyStatus(ctx context.Context, status models.LoopStatus) (
 		}
 		s.SetStatus(newStatus)
 
-		return status.String(), nil
+		return newStatus.String(), nil
 	default:
 		return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s",
 			ErrInvalidStatus, status, constants.Running, constants.Stopped)