From 0df023093990b20b0bcca5ebe8253a0ef025a765 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 1 Mar 2023 03:45:16 -0500 Subject: [PATCH 1/4] fix: Update list of watchers serially to avoid a data race --- internal/generator/generator.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/generator/generator.go b/internal/generator/generator.go index 6f0d08df..0f8d77f4 100644 --- a/internal/generator/generator.go +++ b/internal/generator/generator.go @@ -190,11 +190,11 @@ func (g *generator) generateFromEvents() { } g.wg.Add(1) + watcher := make(chan *docker.APIEvents, 100) + watchers = append(watchers, watcher) - go func(cfg config.Config, watcher chan *docker.APIEvents) { + go func(cfg config.Config) { defer g.wg.Done() - watchers = append(watchers, watcher) - debouncedChan := newDebounceChannel(watcher, cfg.Wait) for range debouncedChan { containers, err := g.getContainers() @@ -210,7 +210,7 @@ func (g *generator) generateFromEvents() { g.runNotifyCmd(cfg) g.sendSignalToContainer(cfg) } - }(cfg, make(chan *docker.APIEvents, 100)) + }(cfg) } // maintains docker client connection and passes events to watchers From 40e53d22bde7ea3b8acf2c7026a794f17ae501fd Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 1 Mar 2023 18:54:37 -0500 Subject: [PATCH 2/4] tests: Use atomic integer for counter to avoid data race --- internal/generator/generator_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go index f82b43fd..c7d7e4fd 100644 --- a/internal/generator/generator_test.go +++ b/internal/generator/generator_test.go @@ -9,6 +9,7 @@ import ( "net/http" "os" "strings" + "sync/atomic" "testing" "time" @@ -22,7 +23,7 @@ import ( func TestGenerateFromEvents(t *testing.T) { log.SetOutput(ioutil.Discard) containerID := "8dfafdbc3a40" - counter := 0 + var counter atomic.Int32 eventsResponse := ` {"status":"start","id":"8dfafdbc3a40","from":"base:latest","time":1374067924} @@ -67,7 +68,7 @@ func TestGenerateFromEvents(t *testing.T) { json.NewEncoder(w).Encode(result) })) server.CustomHandler(fmt.Sprintf("/containers/%s/json", containerID), http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - counter++ + counter := counter.Add(1) container := docker.Container{ Name: "docker-gen-test", ID: containerID, From c8cfb0bbad6949a1d24084948f63b6fe4923f3bf Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 1 Mar 2023 18:59:07 -0500 Subject: [PATCH 3/4] tests: Slow down generator tests to work around race condition 5ms resolution is too fine for reliable tests. Increase delays by a factor of 10 to improve the chances that the desired path will win the race. Ideally the test would be rewritten to remove the races entirely, but I'll leave that as a future exercise. --- internal/generator/generator_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go index c7d7e4fd..9754283f 100644 --- a/internal/generator/generator_test.go +++ b/internal/generator/generator_test.go @@ -39,7 +39,7 @@ func TestGenerateFromEvents(t *testing.T) { for rsc.Scan() { w.Write([]byte(rsc.Text())) w.(http.Flusher).Flush() - time.Sleep(15 * time.Millisecond) + time.Sleep(150 * time.Millisecond) } time.Sleep(500 * time.Millisecond) })) @@ -166,13 +166,13 @@ func TestGenerateFromEvents(t *testing.T) { Template: tmplFile.Name(), Dest: destFiles[2].Name(), Watch: true, - Wait: &config.Wait{Min: 20 * time.Millisecond, Max: 25 * time.Millisecond}, + Wait: &config.Wait{Min: 200 * time.Millisecond, Max: 250 * time.Millisecond}, }, { Template: tmplFile.Name(), Dest: destFiles[3].Name(), Watch: true, - Wait: &config.Wait{Min: 25 * time.Millisecond, Max: 100 * time.Millisecond}, + Wait: &config.Wait{Min: 250 * time.Millisecond, Max: 1 * time.Second}, }, }, }, @@ -189,12 +189,12 @@ func TestGenerateFromEvents(t *testing.T) { // The counter is incremented in each output file in the following sequence: // - // init 0ms 5ms 10ms 15ms 20ms 25ms 30ms 35ms 40ms 45ms 50ms 55ms + // init 150ms 200ms 250ms 300ms 350ms 400ms 450ms 500ms 550ms 600ms 650ms 700ms // ├──────╫──────┼──────┼──────╫──────┼──────┼──────╫──────┼──────┼──────┼──────┼──────┤ // File0 ├─ 1 ║ ║ ║ // File1 ├─ 1 ╟─ 2 ╟─ 3 ╟─ 5 - // File2 ├─ 1 ╟───── max (25ms) ───║───────────> 4 ╟─────── min (20ms) ──────> 6 - // File3 └─ 1 ╟──────────────────> ╟──────────────────> ╟─────────── min (25ms) ─────────> 7 + // File2 ├─ 1 ╟───── max (250ms) ──║───────────> 4 ╟─────── min (200ms) ─────> 6 + // File3 └─ 1 ╟──────────────────> ╟──────────────────> ╟─────────── min (250ms) ────────> 7 // ┌───╨───┐ ┌───╨──┐ ┌───╨───┐ // │ start │ │ stop │ │ start │ // └───────┘ └──────┘ └───────┘ From ab9ed54a35a314640f61c7d584441d42f17375f0 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 1 Mar 2023 19:10:33 -0500 Subject: [PATCH 4/4] tests: Pass `-race` to `go test` to detect data races --- .github/workflows/tests.yml | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index dcd1c2f1..5b53b254 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -38,4 +38,4 @@ jobs: run: make check-gofmt - name: Run tests - run: go test -v ./internal/... + run: go test -race -v ./internal/... diff --git a/Makefile b/Makefile index 307d94c9..475fc433 100644 --- a/Makefile +++ b/Makefile @@ -56,4 +56,4 @@ check-gofmt: fi test: - go test ./internal/... + go test -race ./internal/...