Skip to content

Commit 7aaea28

Browse files
committed
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]>
1 parent 6bedc19 commit 7aaea28

File tree

3 files changed

+12
-11
lines changed

3 files changed

+12
-11
lines changed

pkg/compose/convergence.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,9 @@ func (s *composeService) recreateContainer(ctx context.Context, project *types.P
455455

456456
// setDependentLifecycle define the Lifecycle strategy for all services to depend on specified service
457457
func setDependentLifecycle(project *types.Project, service string, strategy string) {
458+
mu.Lock()
459+
defer mu.Unlock()
460+
458461
for i, s := range project.Services {
459462
if utils.StringContains(s.GetDependencies(), service) {
460463
if s.Extensions == nil {

pkg/compose/watch.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func (s *composeService) Watch(ctx context.Context, project *types.Project, serv
133133
}
134134
ignore := watch.NewCompositeMatcher(
135135
dockerIgnores,
136-
watch.EphemeralPathMatcher,
136+
watch.EphemeralPathMatcher(),
137137
dotGitIgnore,
138138
)
139139

@@ -336,17 +336,17 @@ type rebuildServices map[string]utils.Set[string]
336336

337337
func debounce(ctx context.Context, clock clockwork.Clock, delay time.Duration, input <-chan fileMapping, fn func(services rebuildServices)) {
338338
services := make(rebuildServices)
339-
t := clock.AfterFunc(delay, func() {
340-
if len(services) > 0 {
341-
fn(services)
342-
// TODO(milas): this is a data race!
343-
services = make(rebuildServices)
344-
}
345-
})
339+
t := clock.NewTimer(delay)
340+
defer t.Stop()
346341
for {
347342
select {
348343
case <-ctx.Done():
349344
return
345+
case <-t.Chan():
346+
if len(services) > 0 {
347+
go fn(services)
348+
services = make(rebuildServices)
349+
}
350350
case e := <-input:
351351
t.Reset(delay)
352352
svc, ok := services[e.Service]

pkg/watch/ephemeral.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ package watch
2525
// there or aren't in the right places.
2626
//
2727
// https://app.clubhouse.io/windmill/story/691/filter-out-ephemeral-file-changes
28-
var EphemeralPathMatcher = initEphemeralPathMatcher()
29-
30-
func initEphemeralPathMatcher() PathMatcher {
28+
func EphemeralPathMatcher() PathMatcher {
3129
golandPatterns := []string{"**/*___jb_old___", "**/*___jb_tmp___", "**/.idea/**"}
3230
emacsPatterns := []string{"**/.#*", "**/#*#"}
3331
// if .swp is taken (presumably because multiple vims are running in that dir),

0 commit comments

Comments
 (0)