Skip to content

Commit 01df1b2

Browse files
committed
fix: eliminate data races detected by -race flag
- Add sync.WaitGroup to DockerHandler to track background goroutines; Shutdown now waits for all goroutines to finish before nilling the provider, preventing the race between watchEvents/watchConfig/ watchContainerPolling reads and Shutdown's write to dockerProvider. - Fix TestProgressIndicator_Animate_TerminalMode: wait for the animate goroutine to fully exit before reading the shared bytes.Buffer, replacing the racy time.Sleep with a proper done-channel wait. - Remove t.Parallel() from TestBoot_MissingConfigUsesEmpty, TestBoot_WebAuthEnabled_MissingUsername, and TestBoot_WebAuthEnabled_MissingPassword which all write to the global newDockerHandler variable concurrently. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent 3d8f7f4 commit 01df1b2

File tree

3 files changed

+34
-9
lines changed

3 files changed

+34
-9
lines changed

cli/daemon_cov_ext_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestBoot_InvalidLogLevel(t *testing.T) {
4040
}
4141

4242
func TestBoot_MissingConfigUsesEmpty(t *testing.T) {
43-
t.Parallel()
43+
// Not parallel - modifies global newDockerHandler
4444

4545
logger, handler := test.NewTestLoggerWithHandler()
4646
lv := &slog.LevelVar{}
@@ -73,7 +73,7 @@ func TestBoot_MissingConfigUsesEmpty(t *testing.T) {
7373
}
7474

7575
func TestBoot_WebAuthEnabled_MissingUsername(t *testing.T) {
76-
t.Parallel()
76+
// Not parallel - modifies global newDockerHandler
7777

7878
tmpDir := t.TempDir()
7979
configPath := filepath.Join(tmpDir, "config.ini")
@@ -114,7 +114,7 @@ command = echo test
114114
}
115115

116116
func TestBoot_WebAuthEnabled_MissingPassword(t *testing.T) {
117-
t.Parallel()
117+
// Not parallel - modifies global newDockerHandler
118118

119119
tmpDir := t.TempDir()
120120
configPath := filepath.Join(tmpDir, "config.ini")

cli/docker_config_handler.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ type DockerHandler struct {
4040
fallbackPollingActive bool
4141
fallbackCancel context.CancelFunc // To stop fallback polling when events recover
4242

43+
wg sync.WaitGroup // tracks background goroutines for clean shutdown
44+
4345
includeStopped bool // When true, ListContainers uses All: true so stopped containers are included
4446
}
4547

@@ -132,17 +134,29 @@ func NewDockerHandler(
132134

133135
// Start config file watcher (separate from container detection)
134136
if c.configPollInterval > 0 {
135-
go c.watchConfig()
137+
c.wg.Add(1)
138+
go func() {
139+
defer c.wg.Done()
140+
c.watchConfig()
141+
}()
136142
}
137143

138144
// Start container detection
139145
if c.useEvents {
140-
go c.watchEvents()
146+
c.wg.Add(1)
147+
go func() {
148+
defer c.wg.Done()
149+
c.watchEvents()
150+
}()
141151
}
142152

143153
// Start explicit container polling (if enabled, separate from events)
144154
if c.dockerPollInterval > 0 {
145-
go c.watchContainerPolling()
155+
c.wg.Add(1)
156+
go func() {
157+
defer c.wg.Done()
158+
c.watchContainerPolling()
159+
}()
146160
}
147161

148162
return c, nil
@@ -435,6 +449,9 @@ func (c *DockerHandler) Shutdown(ctx context.Context) error {
435449
c.cancel()
436450
}
437451

452+
// Wait for all background goroutines to finish before closing provider
453+
c.wg.Wait()
454+
438455
// Close SDK provider if it was created
439456
if c.dockerProvider != nil {
440457
if err := c.dockerProvider.Close(); err != nil {

cli/progress_cov_ext_test.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,22 @@ func TestProgressIndicator_Animate_TerminalMode(t *testing.T) {
3232
ticker: ticker,
3333
}
3434

35-
go progress.animate()
35+
animateDone := make(chan struct{})
36+
go func() {
37+
progress.animate()
38+
close(animateDone)
39+
}()
3640

3741
// Let it animate a few frames
3842
time.Sleep(50 * time.Millisecond)
3943
close(done)
4044

41-
// Give goroutine time to exit
42-
time.Sleep(10 * time.Millisecond)
45+
// Wait for animate goroutine to finish before reading buffer
46+
select {
47+
case <-animateDone:
48+
case <-time.After(2 * time.Second):
49+
t.Fatal("animate goroutine did not exit after done was closed")
50+
}
4351

4452
output := buf.String()
4553
assert.NotEmpty(t, output, "animate should have written output")

0 commit comments

Comments
 (0)