From 4949721c0b38430fddebec91f6b1f557e8497ed2 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 20 Nov 2017 16:29:36 +0000 Subject: [PATCH 1/3] lib/model: Trigger a pull when ignore patterns change GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4532 --- lib/model/folder.go | 2 -- lib/model/model.go | 5 ++--- lib/model/rwfolder.go | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/model/folder.go b/lib/model/folder.go index 38da32fa..4206b649 100644 --- a/lib/model/folder.go +++ b/lib/model/folder.go @@ -50,8 +50,6 @@ func (f *folder) DelayScan(next time.Duration) { f.scan.Delay(next) } -func (f *folder) IndexUpdated() {} - func (f *folder) IgnoresUpdated() { if f.FSWatcherEnabled { f.scheduleWatchRestart() diff --git a/lib/model/model.go b/lib/model/model.go index 59798b5a..26ad628f 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -47,9 +47,8 @@ const ( type service interface { BringToFront(string) DelayScan(d time.Duration) - IndexUpdated() // Remote index was updated notification - IgnoresUpdated() // ignore matcher was updated notification - SchedulePull() + IgnoresUpdated() // ignore matcher was updated notification + SchedulePull() // something relevant changed, we should try a pull Jobs() ([]string, []string) // In progress, Queued Scan(subs []string) error Serve() diff --git a/lib/model/rwfolder.go b/lib/model/rwfolder.go index 44dd5e06..32ea0938 100644 --- a/lib/model/rwfolder.go +++ b/lib/model/rwfolder.go @@ -1728,7 +1728,7 @@ func (f *sendReceiveFolder) basePause() time.Duration { func (f *sendReceiveFolder) IgnoresUpdated() { f.folder.IgnoresUpdated() - f.IndexUpdated() + f.SchedulePull() } // A []fileError is sent as part of an event and will be JSON serialized. From 0a50f374db9c9afa5010526f1ff16cf72ceeed48 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 20 Nov 2017 14:54:52 +0100 Subject: [PATCH 2/3] build: More signatures, more better (ref #3420) --- build.go | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/build.go b/build.go index fd44beda..42196534 100644 --- a/build.go +++ b/build.go @@ -1098,27 +1098,39 @@ func macosCodesign(file string) { func windowsCodesign(file string) { st := "signtool.exe" - args := []string{"sign", "/fd", "sha256"} + if path := os.Getenv("CODESIGN_SIGNTOOL"); path != "" { st = path } - if f := os.Getenv("CODESIGN_CERTIFICATE_FILE"); f != "" { - args = append(args, "/f", f) - } - if p := os.Getenv("CODESIGN_CERTIFICATE_PASSWORD"); p != "" { - args = append(args, "/p", p) - } - if tr := os.Getenv("CODESIGN_TIMESTAMP_SERVER"); tr != "" { - args = append(args, "/tr", tr, "/td", "sha256") - } - args = append(args, file) - bs, err := runError(st, args...) - if err != nil { - log.Println("Codesign: signing failed:", string(bs)) - return + for i, algo := range []string{"sha1", "sha256"} { + args := []string{"sign", "/fd", algo} + if f := os.Getenv("CODESIGN_CERTIFICATE_FILE"); f != "" { + args = append(args, "/f", f) + } + if p := os.Getenv("CODESIGN_CERTIFICATE_PASSWORD"); p != "" { + args = append(args, "/p", p) + } + if tr := os.Getenv("CODESIGN_TIMESTAMP_SERVER"); tr != "" { + switch algo { + case "sha256": + args = append(args, "/tr", tr, "/td", algo) + default: + args = append(args, "/t", tr) + } + } + if i > 0 { + args = append(args, "/as") + } + args = append(args, file) + + bs, err := runError(st, args...) + if err != nil { + log.Println("Codesign: signing failed:", string(bs)) + return + } + log.Println("Codesign: successfully signed", file, "using", algo) } - log.Println("Codesign: successfully signed", file) } func metalint() { From 075a699aaede44143213f2fa505ba423bc100105 Mon Sep 17 00:00:00 2001 From: Audrius Butkevicius Date: Tue, 21 Nov 2017 07:25:38 +0000 Subject: [PATCH 3/3] lib/connections: Trust the model to tell us if we are connected This should address issue as described in https://forum.syncthing.net/t/stun-nig-party-with-paused-devices/10942/13 Essentially the model and the connection service goes out of sync in terms of thinking if we are connected or not. Resort to model as being the ultimate source of truth. I can't immediately pin down how this happens, yet some ideas. ConfigSaved happens in separate routine, so it's possbile that we have some sort of device removed yet connection comes in parallel kind of thing. However, in this case the connection exists in the model, and does not exist in the connection service and the only way for the connection to be removed in the connection service is device removal from the config. Given the subject, this might also be related to the device being paused. Also, adds more info to the logs GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4533 --- cmd/syncthing/gui.go | 5 ++-- cmd/syncthing/mocked_model_test.go | 5 ++-- cmd/syncthing/summaryservice.go | 2 +- lib/connections/service.go | 40 ++++++++---------------------- lib/connections/structs.go | 7 +++++- lib/model/model.go | 8 +++--- lib/model/model_test.go | 3 +++ lib/model/rwfolder.go | 2 +- 8 files changed, 31 insertions(+), 41 deletions(-) diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index f9bbe0cb..ea8c63e8 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -26,6 +26,7 @@ import ( "github.com/rcrowley/go-metrics" "github.com/syncthing/syncthing/lib/config" + "github.com/syncthing/syncthing/lib/connections" "github.com/syncthing/syncthing/lib/db" "github.com/syncthing/syncthing/lib/discover" "github.com/syncthing/syncthing/lib/events" @@ -98,7 +99,7 @@ type modelIntf interface { ScanFolders() map[string]error ScanFolderSubdirs(folder string, subs []string) error BringToFront(folder, file string) - ConnectedTo(deviceID protocol.DeviceID) bool + Connection(deviceID protocol.DeviceID) (connections.Connection, bool) GlobalSize(folder string) db.Counts LocalSize(folder string) db.Counts CurrentSequence(folder string) (int64, bool) @@ -1259,7 +1260,7 @@ func (s *apiService) getPeerCompletion(w http.ResponseWriter, r *http.Request) { for _, folder := range s.cfg.Folders() { for _, device := range folder.DeviceIDs() { deviceStr := device.String() - if s.model.ConnectedTo(device) { + if _, ok := s.model.Connection(device); ok { tot[deviceStr] += s.model.Completion(device, folder.ID).CompletionPct } else { tot[deviceStr] = 0 diff --git a/cmd/syncthing/mocked_model_test.go b/cmd/syncthing/mocked_model_test.go index 6aff58cb..096081c0 100644 --- a/cmd/syncthing/mocked_model_test.go +++ b/cmd/syncthing/mocked_model_test.go @@ -9,6 +9,7 @@ package main import ( "time" + "github.com/syncthing/syncthing/lib/connections" "github.com/syncthing/syncthing/lib/db" "github.com/syncthing/syncthing/lib/model" "github.com/syncthing/syncthing/lib/protocol" @@ -91,8 +92,8 @@ func (m *mockedModel) ScanFolderSubdirs(folder string, subs []string) error { func (m *mockedModel) BringToFront(folder, file string) {} -func (m *mockedModel) ConnectedTo(deviceID protocol.DeviceID) bool { - return false +func (m *mockedModel) Connection(deviceID protocol.DeviceID) (connections.Connection, bool) { + return nil, false } func (m *mockedModel) GlobalSize(folder string) db.Counts { diff --git a/cmd/syncthing/summaryservice.go b/cmd/syncthing/summaryservice.go index d62ba36e..58ef3db4 100644 --- a/cmd/syncthing/summaryservice.go +++ b/cmd/syncthing/summaryservice.go @@ -198,7 +198,7 @@ func (c *folderSummaryService) sendSummary(folder string) { // We already know about ourselves. continue } - if !c.model.ConnectedTo(devCfg.DeviceID) { + if _, ok := c.model.Connection(devCfg.DeviceID); !ok { // We're not interested in disconnected devices. continue } diff --git a/lib/connections/service.go b/lib/connections/service.go index 72cd1e79..52e29f6b 100644 --- a/lib/connections/service.go +++ b/lib/connections/service.go @@ -89,9 +89,6 @@ type Service struct { listeners map[string]genericListener listenerTokens map[string]suture.ServiceToken listenerSupervisor *suture.Supervisor - - curConMut sync.Mutex - currentConnection map[protocol.DeviceID]completeConn } func NewService(cfg *config.Wrapper, myID protocol.DeviceID, mdl Model, tlsCfg *tls.Config, discoverer discover.Finder, @@ -129,9 +126,6 @@ func NewService(cfg *config.Wrapper, myID protocol.DeviceID, mdl Model, tlsCfg * FailureThreshold: 2, FailureBackoff: 600 * time.Second, }), - - curConMut: sync.NewMutex(), - currentConnection: make(map[protocol.DeviceID]completeConn), } cfg.Subscribe(service) @@ -228,15 +222,11 @@ next: // If we have a relay connection, and the new incoming connection is // not a relay connection, we should drop that, and prefer this one. - connected := s.model.ConnectedTo(remoteID) - s.curConMut.Lock() - ct, ok := s.currentConnection[remoteID] - s.curConMut.Unlock() - priorityKnown := ok && connected + ct, connected := s.model.Connection(remoteID) // Lower priority is better, just like nice etc. - if priorityKnown && ct.internalConn.priority > c.priority { - l.Debugln("Switching connections", remoteID) + if connected && ct.Priority() > c.priority { + l.Debugf("Switching connections %s (existing: %s new: %s)", remoteID, ct, c) } else if connected { // We should not already be connected to the other party. TODO: This // could use some better handling. If the old connection is dead but @@ -244,7 +234,7 @@ next: // this one. But in case we are two devices connecting to each other // in parallel we don't want to do that or we end up with no // connections still established... - l.Infof("Connected to already connected device (%s)", remoteID) + l.Infof("Connected to already connected device %s (existing: %s new: %s)", remoteID, ct, c) c.Close() continue } @@ -284,9 +274,6 @@ next: l.Infof("Established secure connection to %s at %s (%s)", remoteID, name, tlsCipherSuiteNames[c.ConnectionState().CipherSuite]) s.model.AddConnection(modelConn, hello) - s.curConMut.Lock() - s.currentConnection[remoteID] = modelConn - s.curConMut.Unlock() continue next } } @@ -329,19 +316,13 @@ func (s *Service) connect() { continue } - connected := s.model.ConnectedTo(deviceID) - s.curConMut.Lock() - ct, ok := s.currentConnection[deviceID] - s.curConMut.Unlock() - priorityKnown := ok && connected + ct, connected := s.model.Connection(deviceID) - if priorityKnown && ct.internalConn.priority == bestDialerPrio { + if connected && ct.Priority() == bestDialerPrio { // Things are already as good as they can get. continue } - l.Debugln("Reconnect loop for", deviceID) - var addrs []string for _, addr := range deviceCfg.Addresses { if addr == "dynamic" { @@ -355,6 +336,8 @@ func (s *Service) connect() { } } + l.Debugln("Reconnect loop for", deviceID, addrs) + seen = append(seen, addrs...) dialTargets := make([]dialTarget, 0) @@ -394,8 +377,8 @@ func (s *Service) connect() { prio := dialerFactory.Priority() - if priorityKnown && prio >= ct.internalConn.priority { - l.Debugf("Not dialing using %s as priority is less than current connection (%d >= %d)", dialerFactory, dialerFactory.Priority(), ct.internalConn.priority) + if connected && prio >= ct.Priority() { + l.Debugf("Not dialing using %s as priority is less than current connection (%d >= %d)", dialerFactory, dialerFactory.Priority(), ct.Priority()) continue } @@ -492,9 +475,6 @@ func (s *Service) CommitConfiguration(from, to config.Configuration) bool { for _, dev := range from.Devices { if !newDevices[dev.DeviceID] { - s.curConMut.Lock() - delete(s.currentConnection, dev.DeviceID) - s.curConMut.Unlock() warningLimitersMut.Lock() delete(warningLimiters, dev.DeviceID) warningLimitersMut.Unlock() diff --git a/lib/connections/structs.go b/lib/connections/structs.go index 0dfad5b0..7df84f76 100644 --- a/lib/connections/structs.go +++ b/lib/connections/structs.go @@ -27,6 +27,7 @@ type Connection interface { Type() string Transport() string RemoteAddr() net.Addr + Priority() int } // completeConn is the aggregation of an internalConn and the @@ -92,6 +93,10 @@ func (c internalConn) Type() string { return c.connType.String() } +func (c internalConn) Priority() int { + return c.priority +} + func (c internalConn) Transport() string { transport := c.connType.Transport() host, _, err := net.SplitHostPort(c.LocalAddr().String()) @@ -152,7 +157,7 @@ type genericListener interface { type Model interface { protocol.Model AddConnection(conn Connection, hello protocol.HelloResult) - ConnectedTo(remoteID protocol.DeviceID) bool + Connection(remoteID protocol.DeviceID) (Connection, bool) OnHello(protocol.DeviceID, net.Addr, protocol.HelloResult) error GetHello(protocol.DeviceID) protocol.HelloIntf } diff --git a/lib/model/model.go b/lib/model/model.go index 26ad628f..be9fbcfb 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -1330,15 +1330,15 @@ func (cf cFiler) CurrentFile(file string) (protocol.FileInfo, bool) { return cf.m.CurrentFolderFile(cf.r, file) } -// ConnectedTo returns true if we are connected to the named device. -func (m *Model) ConnectedTo(deviceID protocol.DeviceID) bool { +// Connection returns the current connection for device, and a boolean wether a connection was found. +func (m *Model) Connection(deviceID protocol.DeviceID) (connections.Connection, bool) { m.pmut.RLock() - _, ok := m.conn[deviceID] + cn, ok := m.conn[deviceID] m.pmut.RUnlock() if ok { m.deviceWasSeen(deviceID) } - return ok + return cn, ok } func (m *Model) GetIgnores(folder string) ([]string, []string, error) { diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 25cba823..e167fd12 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -311,6 +311,9 @@ func (f *fakeConnection) Type() string { func (f *fakeConnection) Transport() string { return "fake" } +func (f *fakeConnection) Priority() int { + return 9000 +} func (f *fakeConnection) DownloadProgress(folder string, updates []protocol.FileDownloadProgressUpdate) { f.downloadProgressMessages = append(f.downloadProgressMessages, downloadProgressMessage{ diff --git a/lib/model/rwfolder.go b/lib/model/rwfolder.go index 32ea0938..dcaf3399 100644 --- a/lib/model/rwfolder.go +++ b/lib/model/rwfolder.go @@ -399,7 +399,7 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher, ignoresChan devices := folderFiles.Availability(file.Name) for _, dev := range devices { - if f.model.ConnectedTo(dev) { + if _, ok := f.model.Connection(dev); ok { f.queue.Push(file.Name, file.Size, file.ModTime()) changed++ return true