From 66662cd67817514648050fbdb579e056a6dc61f7 Mon Sep 17 00:00:00 2001
From: Gahl Saraf <saraf.gahl@gmail.com>
Date: Mon, 26 Apr 2021 23:13:59 +0300
Subject: [PATCH] Trigger connection loop on config device addition (fixes
 #7600) (#7604)

* Trigger connection loop on config device addition (fixes #7600)

* Also check for device address equality

* Move EqualStrings from api_test to utils, and use in connections/service.go

* Make sure CommitConfiguration cannot block due on the deviceAddressesChanged channel

* Update lib/connections/service.go

Co-authored-by: Jakob Borg <jakob@kastelo.net>
---
 lib/api/api_test.go        | 16 +++-------------
 lib/connections/service.go | 38 +++++++++++++++++++++++++++++++-------
 lib/util/utils.go          | 12 ++++++++++++
 3 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/lib/api/api_test.go b/lib/api/api_test.go
index c20296a7d..951776d7e 100644
--- a/lib/api/api_test.go
+++ b/lib/api/api_test.go
@@ -42,6 +42,7 @@ import (
 	"github.com/syncthing/syncthing/lib/sync"
 	"github.com/syncthing/syncthing/lib/tlsutil"
 	"github.com/syncthing/syncthing/lib/ur"
+	"github.com/syncthing/syncthing/lib/util"
 	"github.com/thejerf/suture/v4"
 )
 
@@ -126,6 +127,7 @@ func TestStopAfterBrokenConfig(t *testing.T) {
 
 	srv := New(protocol.LocalDeviceID, w, "", "syncthing", nil, nil, nil, events.NoopLogger, nil, nil, nil, nil, nil, nil, false).(*service)
 	defer os.Remove(token)
+
 	srv.started = make(chan string)
 
 	sup := suture.New("test", svcutil.SpecWithDebugLogger(l))
@@ -1178,7 +1180,7 @@ func TestBrowse(t *testing.T) {
 
 	for _, tc := range cases {
 		ret := browseFiles(tc.current, fs.FilesystemTypeBasic)
-		if !equalStrings(ret, tc.returns) {
+		if !util.EqualStrings(ret, tc.returns) {
 			t.Errorf("browseFiles(%q) => %q, expected %q", tc.current, ret, tc.returns)
 		}
 	}
@@ -1389,18 +1391,6 @@ func TestSanitizedHostname(t *testing.T) {
 	}
 }
 
-func equalStrings(a, b []string) bool {
-	if len(a) != len(b) {
-		return false
-	}
-	for i := range a {
-		if a[i] != b[i] {
-			return false
-		}
-	}
-	return true
-}
-
 // runningInContainer returns true if we are inside Docker or LXC. It might
 // be prone to false negatives if things change in the future, but likely
 // not false positives.
diff --git a/lib/connections/service.go b/lib/connections/service.go
index fa0ee7093..9ecefebd0 100644
--- a/lib/connections/service.go
+++ b/lib/connections/service.go
@@ -141,10 +141,11 @@ type service struct {
 	natService           *nat.Service
 	evLogger             events.Logger
 
-	listenersMut       sync.RWMutex
-	listeners          map[string]genericListener
-	listenerTokens     map[string]suture.ServiceToken
-	listenerSupervisor *suture.Supervisor
+	deviceAddressesChanged chan struct{}
+	listenersMut           sync.RWMutex
+	listeners              map[string]genericListener
+	listenerTokens         map[string]suture.ServiceToken
+	listenerSupervisor     *suture.Supervisor
 }
 
 func NewService(cfg config.Wrapper, myID protocol.DeviceID, mdl Model, tlsCfg *tls.Config, discoverer discover.Finder, bepProtocolName string, tlsDefaultCommonName string, evLogger events.Logger) Service {
@@ -165,9 +166,10 @@ func NewService(cfg config.Wrapper, myID protocol.DeviceID, mdl Model, tlsCfg *t
 		natService:           nat.NewService(myID, cfg),
 		evLogger:             evLogger,
 
-		listenersMut:   sync.NewRWMutex(),
-		listeners:      make(map[string]genericListener),
-		listenerTokens: make(map[string]suture.ServiceToken),
+		deviceAddressesChanged: make(chan struct{}, 1),
+		listenersMut:           sync.NewRWMutex(),
+		listeners:              make(map[string]genericListener),
+		listenerTokens:         make(map[string]suture.ServiceToken),
 
 		// A listener can fail twice, rapidly. Any more than that and it
 		// will be put on suspension for ten minutes. Restarts and changes
@@ -392,6 +394,7 @@ func (s *service) connect(ctx context.Context) error {
 		l.Debugln("Next connection loop in", sleep)
 
 		select {
+		case <-s.deviceAddressesChanged:
 		case <-time.After(sleep):
 		case <-ctx.Done():
 			return ctx.Err()
@@ -675,6 +678,8 @@ func (s *service) CommitConfiguration(from, to config.Configuration) bool {
 		}
 	}
 
+	s.checkAndSignalConnectLoopOnUpdatedDevices(from, to)
+
 	s.listenersMut.Lock()
 	seen := make(map[string]struct{})
 	for _, addr := range to.Options.ListenAddresses() {
@@ -733,6 +738,25 @@ func (s *service) CommitConfiguration(from, to config.Configuration) bool {
 	return true
 }
 
+func (s *service) checkAndSignalConnectLoopOnUpdatedDevices(from, to config.Configuration) {
+	oldDevices := make(map[protocol.DeviceID]config.DeviceConfiguration, len(from.Devices))
+	for _, dev := range from.Devices {
+		oldDevices[dev.DeviceID] = dev
+	}
+
+	for _, dev := range to.Devices {
+		oldDev, ok := oldDevices[dev.DeviceID]
+		if !ok || !util.EqualStrings(oldDev.Addresses, dev.Addresses) {
+			select {
+			case s.deviceAddressesChanged <- struct{}{}:
+			default:
+				// channel is blocked - a config update is already pending for the connection loop.
+			}
+			break
+		}
+	}
+}
+
 func (s *service) AllAddresses() []string {
 	s.listenersMut.RLock()
 	var addrs []string
diff --git a/lib/util/utils.go b/lib/util/utils.go
index 427ace285..57b317d28 100644
--- a/lib/util/utils.go
+++ b/lib/util/utils.go
@@ -280,3 +280,15 @@ func NiceDurationString(d time.Duration) string {
 	}
 	return d.String()
 }
+
+func EqualStrings(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}