Repository: syncthing/syncthing (language: go)

Issue:
panic: nil pointer dereference in (*indexHandlerRegistry).startLocked
```
2023/12/07 21:27:41 INFO: syncthing v1.27.0 "Gold Grasshopper" (go1.20.11 linux-amd64) root@buildkitsandbox 2023-12-05 07:12:09 UTC [noupgrade]
Panic at 2023-12-07T21:35:02+01:00
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x60 pc=0xd61b6e]

goroutine 171032 [running]:
github.com/syncthing/syncthing/lib/model.(*indexHandlerRegistry).startLocked(_, {{0xc00512dbc0, 0xb}, {0xc00512dbd0, 0xe}, 0x0, {0xc00116e9e0, 0x20}, 0x0, {0xc000e1bba0, ...}, ...}, ...)
	github.com/syncthing/syncthing/lib/model/indexhandler.go:419 +0x16e
github.com/syncthing/syncthing/lib/model.(*indexHandlerRegistry).AddIndexInfo(0xc0015e9400, {0xc0009d0ae0, 0xb}, 0xc00175a500)
	github.com/syncthing/syncthing/lib/model/indexhandler.go:439 +0x4b5
github.com/syncthing/syncthing/lib/model.(*model).ccHandleFolders(_, {_, _, _}, {{0x81, 0xc1, 0x8d, 0x7b, 0xd9, 0x97, ...}, ...}, ...)
	github.com/syncthing/syncthing/lib/model/model.go:1431 +0xca6
github.com/syncthing/syncthing/lib/model.(*model).ClusterConfig(0xc000002000, {0x13c2688, 0xc000fe6170}, {{0xc0011ac000?, 0xc0000d0088?, 0xc001429d78?}, 0xcf?})
	github.com/syncthing/syncthing/lib/model/model.go:1296 +0x8e8
github.com/syncthing/syncthing/lib/protocol.(*connectionWrappingModel).ClusterConfig(0xc001429d78?, {{0xc0011ac000?, 0x10?, 0xe96c40?}, 0x1?})
	github.com/syncthing/syncthing/lib/protocol/protocol.go:1143 +0x43
github.com/syncthing/syncthing/lib/protocol.encryptedModel.ClusterConfig(...)
	github.com/syncthing/syncthing/lib/protocol/encryption.go:155
github.com/syncthing/syncthing/lib/protocol.(*rawConnection).dispatcherLoop(0xc000d9b9e0)
	github.com/syncthing/syncthing/lib/protocol/protocol.go:496 +0x3b7
github.com/syncthing/syncthing/lib/protocol.(*rawConnection).Start.func2()
	github.com/syncthing/syncthing/lib/protocol/protocol.go:307 +0x25
created by github.com/syncthing/syncthing/lib/protocol.(*rawConnection).Start
	github.com/syncthing/syncthing/lib/protocol/protocol.go:306 +0x116
```

`runner` is nil here:

https://github.com/syncthing/syncthing/blob/75310b58a06605055823826b4e63e4356ce2e7c5/lib/model/indexhandler.go#L419

//@calmh


Discussion / hints:
Same as #9234 which we thought fixed.
Obvious culprits are 51185381797a6309c5f579d4de6d8ba4dd9e1e76 and c6334e61aa56575db6b274d45d55f7ada4dcd0fe where the latter added the incorrect locking subsequently fixed, but which was apparently not the root cause...

Fix patch:
diff --git a/lib/model/model.go b/lib/model/model.go
index 4cc94238c1a..878ef9d736f 100644
--- a/lib/model/model.go
+++ b/lib/model/model.go
@@ -465,7 +465,7 @@ func (m *model) warnAboutOverwritingProtectedFiles(cfg config.FolderConfiguratio
 
 func (m *model) removeFolder(cfg config.FolderConfiguration) {
 	m.fmut.RLock()
-	wait := m.folderRunners.RemoveAndWaitChan(cfg.ID, 0)
+	wait := m.folderRunners.StopAndWaitChan(cfg.ID, 0)
 	m.fmut.RUnlock()
 	<-wait
 
@@ -507,6 +507,7 @@ func (m *model) removeFolder(cfg config.FolderConfiguration) {
 // Need to hold lock on m.fmut when calling this.
 func (m *model) cleanupFolderLocked(cfg config.FolderConfiguration) {
 	// clear up our config maps
+	m.folderRunners.Remove(cfg.ID)
 	delete(m.folderCfgs, cfg.ID)
 	delete(m.folderFiles, cfg.ID)
 	delete(m.folderIgnores, cfg.ID)
@@ -536,7 +537,7 @@ func (m *model) restartFolder(from, to config.FolderConfiguration, cacheIgnoredF
 	defer restartMut.Unlock()
 
 	m.fmut.RLock()
-	wait := m.folderRunners.RemoveAndWaitChan(from.ID, 0)
+	wait := m.folderRunners.StopAndWaitChan(from.ID, 0)
 	m.fmut.RUnlock()
 	<-wait
 
diff --git a/lib/model/service_map.go b/lib/model/service_map.go
index 988ad520ae9..0eed99cfc2b 100644
--- a/lib/model/service_map.go
+++ b/lib/model/service_map.go
@@ -59,6 +59,34 @@ func (s *serviceMap[K, S]) Get(k K) (v S, ok bool) {
 	return
 }
 
+// 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 {
+		s.supervisor.Remove(tok)
+	}
+	return
+}
+
+// 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 {
+		go func() {
+			ret <- s.supervisor.RemoveAndWait(tok, timeout)
+		}()
+	} else {
+		ret <- errSvcNotFound
+	}
+	return ret
+}
+
 // Remove removes the service at the given key, stopping it on the supervisor.
 // If there is no service at the given key, nothing happens. The return value
 // indicates whether a service was removed.
@@ -66,6 +94,8 @@ func (s *serviceMap[K, S]) Remove(k K) (found bool) {
 	if tok, ok := s.tokens[k]; ok {
 		found = true
 		s.supervisor.Remove(tok)
+	} else {
+		_, found = s.services[k]
 	}
 	delete(s.services, k)
 	delete(s.tokens, k)
@@ -84,16 +114,8 @@ func (s *serviceMap[K, S]) RemoveAndWait(k K, timeout time.Duration) error {
 // value: either the return value from RemoveAndWait (possibly nil), or
 // errSvcNotFound if the service was not found.
 func (s *serviceMap[K, S]) RemoveAndWaitChan(k K, timeout time.Duration) <-chan error {
-	ret := make(chan error, 1)
-	if tok, ok := s.tokens[k]; ok {
-		go func() {
-			ret <- s.supervisor.RemoveAndWait(tok, timeout)
-		}()
-	} else {
-		ret <- errSvcNotFound
-	}
+	ret := s.StopAndWaitChan(k, timeout)
 	delete(s.services, k)
-	delete(s.tokens, k)
 	return ret
 }
 


Test patch: