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>
This commit is contained in:
Gahl Saraf 2021-04-26 23:13:59 +03:00 committed by GitHub
parent 8734fa65fc
commit 66662cd678
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 20 deletions

View File

@ -42,6 +42,7 @@ import (
"github.com/syncthing/syncthing/lib/sync" "github.com/syncthing/syncthing/lib/sync"
"github.com/syncthing/syncthing/lib/tlsutil" "github.com/syncthing/syncthing/lib/tlsutil"
"github.com/syncthing/syncthing/lib/ur" "github.com/syncthing/syncthing/lib/ur"
"github.com/syncthing/syncthing/lib/util"
"github.com/thejerf/suture/v4" "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) srv := New(protocol.LocalDeviceID, w, "", "syncthing", nil, nil, nil, events.NoopLogger, nil, nil, nil, nil, nil, nil, false).(*service)
defer os.Remove(token) defer os.Remove(token)
srv.started = make(chan string) srv.started = make(chan string)
sup := suture.New("test", svcutil.SpecWithDebugLogger(l)) sup := suture.New("test", svcutil.SpecWithDebugLogger(l))
@ -1178,7 +1180,7 @@ func TestBrowse(t *testing.T) {
for _, tc := range cases { for _, tc := range cases {
ret := browseFiles(tc.current, fs.FilesystemTypeBasic) 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) 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 // 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 // be prone to false negatives if things change in the future, but likely
// not false positives. // not false positives.

View File

@ -141,10 +141,11 @@ type service struct {
natService *nat.Service natService *nat.Service
evLogger events.Logger evLogger events.Logger
listenersMut sync.RWMutex deviceAddressesChanged chan struct{}
listeners map[string]genericListener listenersMut sync.RWMutex
listenerTokens map[string]suture.ServiceToken listeners map[string]genericListener
listenerSupervisor *suture.Supervisor 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 { 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), natService: nat.NewService(myID, cfg),
evLogger: evLogger, evLogger: evLogger,
listenersMut: sync.NewRWMutex(), deviceAddressesChanged: make(chan struct{}, 1),
listeners: make(map[string]genericListener), listenersMut: sync.NewRWMutex(),
listenerTokens: make(map[string]suture.ServiceToken), listeners: make(map[string]genericListener),
listenerTokens: make(map[string]suture.ServiceToken),
// A listener can fail twice, rapidly. Any more than that and it // A listener can fail twice, rapidly. Any more than that and it
// will be put on suspension for ten minutes. Restarts and changes // 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) l.Debugln("Next connection loop in", sleep)
select { select {
case <-s.deviceAddressesChanged:
case <-time.After(sleep): case <-time.After(sleep):
case <-ctx.Done(): case <-ctx.Done():
return ctx.Err() return ctx.Err()
@ -675,6 +678,8 @@ func (s *service) CommitConfiguration(from, to config.Configuration) bool {
} }
} }
s.checkAndSignalConnectLoopOnUpdatedDevices(from, to)
s.listenersMut.Lock() s.listenersMut.Lock()
seen := make(map[string]struct{}) seen := make(map[string]struct{})
for _, addr := range to.Options.ListenAddresses() { for _, addr := range to.Options.ListenAddresses() {
@ -733,6 +738,25 @@ func (s *service) CommitConfiguration(from, to config.Configuration) bool {
return true 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 { func (s *service) AllAddresses() []string {
s.listenersMut.RLock() s.listenersMut.RLock()
var addrs []string var addrs []string

View File

@ -280,3 +280,15 @@ func NiceDurationString(d time.Duration) string {
} }
return d.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
}