Skip to content

fix: Update list of watchers serially to avoid a data race #502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ jobs:
run: make check-gofmt

- name: Run tests
run: go test -v ./internal/...
run: go test -race -v ./internal/...
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,4 @@ check-gofmt:
fi

test:
go test ./internal/...
go test -race ./internal/...
8 changes: 4 additions & 4 deletions internal/generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
17 changes: 9 additions & 8 deletions internal/generator/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"os"
"strings"
"sync/atomic"
"testing"
"time"

Expand All @@ -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}
Expand All @@ -38,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)
}))
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -165,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},
},
},
},
Expand All @@ -188,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 │
// └───────┘ └──────┘ └───────┘
Expand Down