From 7aaea283caf61dc89eadd417c3535784705c7918 Mon Sep 17 00:00:00 2001
From: Milas Bowman <milas.bowman@docker.com>
Date: Wed, 22 Mar 2023 18:05:56 -0400
Subject: [PATCH] watch: data race / segfault fixes

Was getting segfaults with multiple services using
`x-develop` and `watch` at the same time. Turns out
the Moby path matcher lazily initializes the regex
pattern internally the first time it's used, so it's
not goroutine-safe.

Change here is to not use a global instance for the
ephemeral path matcher, but a per-watcher instance.

Additionally, the data race detector caught a couple
other issues that were easy enough to fix:
 * Use the lock that's used elsewhere for convergence
   before manipulating
 * Eliminate concurrent map access when triggering
   rebuilds

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
---
 pkg/compose/convergence.go |  3 +++
 pkg/compose/watch.go       | 16 ++++++++--------
 pkg/watch/ephemeral.go     |  4 +---
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go
index 5fbebd4c7..eda1b45d2 100644
--- a/pkg/compose/convergence.go
+++ b/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 {
diff --git a/pkg/compose/watch.go b/pkg/compose/watch.go
index 00d60bd1b..79b7e2d5b 100644
--- a/pkg/compose/watch.go
+++ b/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]
diff --git a/pkg/watch/ephemeral.go b/pkg/watch/ephemeral.go
index 2aa271503..98538abe5 100644
--- a/pkg/watch/ephemeral.go
+++ b/pkg/watch/ephemeral.go
@@ -25,9 +25,7 @@ package watch
 // 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 {
+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),