Merge pull request docker#10393 from milas/fix-watch-segfault

watch: data race / segfault fixes

Author: milas

pkg/compose/convergence.go

@@ -455,6 +455,9 @@ func (s *composeService) recreateContainer(ctx context.Context, project *types.P
 
 // setDependentLifecycle define the Lifecycle strategy for all services to depend on specified service
 func setDependentLifecycle(project *types.Project, service string, strategy string) {
+	mu.Lock()
+	defer mu.Unlock()
+
 	for i, s := range project.Services {
 		if utils.StringContains(s.GetDependencies(), service) {
 			if s.Extensions == nil {

pkg/compose/watch.go

@@ -133,7 +133,7 @@ func (s *composeService) Watch(ctx context.Context, project *types.Project, serv
 		}
 		ignore := watch.NewCompositeMatcher(
 			dockerIgnores,
-			watch.EphemeralPathMatcher,
+			watch.EphemeralPathMatcher(),
 			dotGitIgnore,
 		)
 
@@ -336,17 +336,17 @@ type rebuildServices map[string]utils.Set[string]
 
 func debounce(ctx context.Context, clock clockwork.Clock, delay time.Duration, input <-chan fileMapping, fn func(services rebuildServices)) {
 	services := make(rebuildServices)
-	t := clock.AfterFunc(delay, func() {
-		if len(services) > 0 {
-			fn(services)
-			// TODO(milas): this is a data race!
-			services = make(rebuildServices)
-		}
-	})
+	t := clock.NewTimer(delay)
+	defer t.Stop()
 	for {
 		select {
 		case <-ctx.Done():
 			return
+		case <-t.Chan():
+			if len(services) > 0 {
+				go fn(services)
+				services = make(rebuildServices)
+			}
 		case e := <-input:
 			t.Reset(delay)
 			svc, ok := services[e.Service]

pkg/watch/ephemeral.go

@@ -24,10 +24,9 @@ package watch
 // stop-gap so they don't have a terrible experience if those files aren't
 // there or aren't in the right places.
 //
-// https://app.clubhouse.io/windmill/story/691/filter-out-ephemeral-file-changes
-var EphemeralPathMatcher = initEphemeralPathMatcher()
-
-func initEphemeralPathMatcher() PathMatcher {
+// NOTE: The underlying `patternmatcher` is NOT always Goroutine-safe, so
+// this is not a singleton; we create an instance for each watcher currently.
+func EphemeralPathMatcher() PathMatcher {
 	golandPatterns := []string{"**/*___jb_old___", "**/*___jb_tmp___", "**/.idea/**"}
 	emacsPatterns := []string{"**/.#*", "**/#*#"}
 	// if .swp is taken (presumably because multiple vims are running in that dir),

pkg/watch/ephemeral_test.go

@@ -0,0 +1,49 @@
+/*
+Copyright 2023 Docker Compose CLI authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package watch_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/docker/compose/v2/pkg/watch"
+)
+
+func TestEphemeralPathMatcher(t *testing.T) {
+	ignored := []string{
+		".file.txt.swp",
+		"/path/file.txt~",
+		"/home/moby/proj/.idea/modules.xml",
+		".#file.txt",
+		"#file.txt#",
+		"/dir/.file.txt.kate-swp",
+		"/go/app/1234-go-tmp-umask",
+	}
+	matcher := watch.EphemeralPathMatcher()
+	for _, p := range ignored {
+		ok, err := matcher.Matches(p)
+		if assert.NoErrorf(t, err, "Matching %s", p) {
+			assert.Truef(t, ok, "Path %s should have matched", p)
+		}
+	}
+
+	const includedPath = "normal.txt"
+	ok, err := matcher.Matches(includedPath)
+	if assert.NoErrorf(t, err, "Matching %s", includedPath) {
+		assert.Falsef(t, ok, "Path %s should NOT have matched", includedPath)
+	}
+}