Skip to content

Commit

Permalink
lib/model: Remove runner during folder cleanup (fixes syncthing#9269)
Browse files Browse the repository at this point in the history
Before introducing the service map and using it for folder runners, the
entries in folderCfgs and folderRunners for the same key/folder were
removed under a single lock. Stopping the folder happens separately
before that with just the read lock. Now with the service map stopping the
folder and removing it from the map is a single operation. And that
still happens with just a read-lock. However even with a full lock it’s still
problematic: After the folder stopped, the runner isn’t present anymore while
the folder-config still is and sais the folder isn't paused.

The index handler in turn looks at the folder config that is not paused,
thus assumes the runner has to be present -> nil deref on the runner.

A better solution would like be to push most of these fmut maps into the
folder - they anyway are needed in there. Then there's just a single
map/source of info that's necessarily consistent.
  • Loading branch information
imsodin committed Dec 7, 2023
1 parent 994e14e commit 87b724d
Showing 1 changed file with 10 additions and 11 deletions.
21 changes: 10 additions & 11 deletions lib/model/service_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,22 @@ func (s *serviceMap[K, S]) Get(k K) (v S, ok bool) {
return
}

// Stop removes the service from the supervisor, stopping it. The service itself
// is still retained, i.e. a call to Get with the same key will still return a
// result.
func (s *serviceMap[K, S]) Stop(k K) (found bool) {
// Stop removes the service at the given key from the supervisor, stopping it.
// The service itself is still retained, i.e. a call to Get with the same key
// will still return a result.
func (s *serviceMap[K, S]) Stop(k K) {
if tok, ok := s.tokens[k]; ok {
found = true
s.supervisor.Remove(tok)
}
delete(s.tokens, k)
return
}

// RemoveAndWaitChan removes the service at the given key, stopping it on
// the supervisor. The returned channel will produce precisely one error
// value: either the return value from RemoveAndWait (possibly nil), or
// errSvcNotFound if the service was not found.
// StopAndWaitChan removes the service at the given key from the supervisor,
// stopping it. The service itself is still retained, i.e. a call to Get with
// the same key will still return a result.
// The returned channel will produce precisely one error value: either the
// return value from RemoveAndWait (possibly nil), or errSvcNotFound if the
// service was not found.
func (s *serviceMap[K, S]) StopAndWaitChan(k K, timeout time.Duration) <-chan error {
ret := make(chan error, 1)
if tok, ok := s.tokens[k]; ok {
Expand All @@ -84,7 +84,6 @@ func (s *serviceMap[K, S]) StopAndWaitChan(k K, timeout time.Duration) <-chan er
} else {
ret <- errSvcNotFound
}
delete(s.tokens, k)
return ret
}

Expand Down

0 comments on commit 87b724d

Please sign in to comment.