Skip to content

Commit

Permalink
Always call wg.Add(1) before starting goroutine
Browse files Browse the repository at this point in the history
While it felt prettier to do both wg.Add(1) and wg.Done() inside the
goroutine functions this is inherently causing a race between starting
the goroutine and the later call to wg.Wait().

While things seems to have worked fine this way lets do it correctly
everywhere to not lead by poor example for future work.
  • Loading branch information
eest committed Sep 3, 2024
1 parent 87dcf90 commit 2dc0525
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions pkg/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ func (edm *dnstapMinimiser) reverseLabelsBounded(labels []string, maxLen int) []

func (edm *dnstapMinimiser) diskCleaner(wg *sync.WaitGroup, sentDir string) {
// We will scan the directory each tick for sent files to remove.
wg.Add(1)
defer wg.Done()

ticker := time.NewTicker(time.Second * 60)
Expand Down Expand Up @@ -682,15 +681,20 @@ func Run() {
labelLimit := 10

// Start record writers and data senders in the background
wg.Add(1)
go edm.sessionWriter(dataDir, &wg)
wg.Add(1)
go edm.histogramWriter(labelLimit, outboxDir, &wg)
if !edm.histogramSenderDisabled {
wg.Add(1)
go edm.histogramSender(outboxDir, sentDir, &wg)
}
if !edm.mqttDisabled {
wg.Add(1)
go edm.newQnamePublisher(&wg)
}

wg.Add(1)
go edm.diskCleaner(&wg, sentDir)

dawgFile := viper.GetString("well-known-domains")
Expand Down Expand Up @@ -733,6 +737,7 @@ func Run() {
}

// Start data collector
wg.Add(1)
go edm.dataCollector(&wg, wkdTracker, dawgFile)

var minimiserWg sync.WaitGroup
Expand Down Expand Up @@ -978,7 +983,6 @@ func (wkd *wellKnownDomainsTracker) lookup(ipBytes []byte, msg *dns.Msg) (int, b
}

func (wkd *wellKnownDomainsTracker) updateRetryer(edm *dnstapMinimiser, wg *sync.WaitGroup) {
wg.Add(1)
defer wg.Done()

for wu := range wkd.retryCh {
Expand Down Expand Up @@ -1345,7 +1349,6 @@ func (edm *dnstapMinimiser) newSession(dt *dnstap.Dnstap, msg *dns.Msg, isQuery
}

func (edm *dnstapMinimiser) sessionWriter(dataDir string, wg *sync.WaitGroup) {
wg.Add(1)
defer wg.Done()

edm.log.Info("sessionStructWriter: starting")
Expand All @@ -1361,7 +1364,6 @@ func (edm *dnstapMinimiser) sessionWriter(dataDir string, wg *sync.WaitGroup) {
}

func (edm *dnstapMinimiser) histogramWriter(labelLimit int, outboxDir string, wg *sync.WaitGroup) {
wg.Add(1)
defer wg.Done()

edm.log.Info("histogramWriter: starting")
Expand Down Expand Up @@ -1436,7 +1438,6 @@ func (edm *dnstapMinimiser) createFile(dst string) (*os.File, error) {
}

func (edm *dnstapMinimiser) histogramSender(outboxDir string, sentDir string, wg *sync.WaitGroup) {
wg.Add(1)
defer wg.Done()

edm.log.Info("histogramSender: starting")
Expand Down Expand Up @@ -1509,7 +1510,6 @@ func timestampsFromFilename(name string) (time.Time, time.Time, error) {
}

func (edm *dnstapMinimiser) newQnamePublisher(wg *sync.WaitGroup) {
wg.Add(1)
defer wg.Done()

edm.log.Info("newQnamePublisher: starting")
Expand Down Expand Up @@ -1918,7 +1918,6 @@ func timeUntilNextMinute() time.Duration {

// runMinimiser generates data and it is collected into datasets here
func (edm *dnstapMinimiser) dataCollector(wg *sync.WaitGroup, wkd *wellKnownDomainsTracker, dawgFile string) {
wg.Add(1)
defer wg.Done()

// Keep track of if we have recorded any dnstap packets in session data
Expand All @@ -1927,6 +1926,7 @@ func (edm *dnstapMinimiser) dataCollector(wg *sync.WaitGroup, wkd *wellKnownDoma
// Start retryer, handles instances where the received update has a
// dawgModTime that is no longer valid becuase it has been rotated.
var retryerWg sync.WaitGroup
retryerWg.Add(1)
go wkd.updateRetryer(edm, &retryerWg)

sessions := []*sessionData{}
Expand Down

0 comments on commit 2dc0525

Please sign in to comment.