Browse Source

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 <[email protected]>
Milas Bowman 2 years ago
parent
commit
7aaea283ca
3 changed files with 12 additions and 11 deletions
  1. 3 0
      pkg/compose/convergence.go
  2. 8 8
      pkg/compose/watch.go
  3. 1 3
      pkg/watch/ephemeral.go

+ 3 - 0
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 {

+ 8 - 8
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]

+ 1 - 3
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),