Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Commit

Permalink
Fix race in RetrieveData
Browse files Browse the repository at this point in the history
RetrieveData previously would return pointers to internal
aggregators that might subsequently be mutated based on future
view updates.

Instead, have collectedRows always return an immutable snapshot
of the collected aggregators (by calling clone on each one).

Since we do this, we can also avoid a defensive copy on the normal
export path.

Fixes: #795
  • Loading branch information
Ramon Nogueira committed Jun 21, 2018
1 parent 7f3f807 commit f427a1a
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 16 deletions.
5 changes: 3 additions & 2 deletions stats/view/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,12 @@ func (c *collector) addSample(s string, v float64) {
aggregator.addSample(v)
}

// collectRows returns a snapshot of the collected Row values.
func (c *collector) collectedRows(keys []tag.Key) []*Row {
var rows []*Row
rows := make([]*Row, 0, len(c.signatures))
for sig, aggregator := range c.signatures {
tags := decodeTags([]byte(sig), keys)
row := &Row{tags, aggregator}
row := &Row{Tags: tags, Data: aggregator.clone()}
rows = append(rows, row)
}
return rows
Expand Down
14 changes: 0 additions & 14 deletions stats/view/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,6 @@ func (w *worker) reportUsage(now time.Time) {
if !ok {
w.startTimes[v] = now
}
// Make sure collector is never going
// to mutate the exported data.
rows = deepCopyRowData(rows)
viewData := &Data{
View: v.view,
Start: w.startTimes[v],
Expand All @@ -220,14 +217,3 @@ func (w *worker) reportUsage(now time.Time) {
exportersMu.Unlock()
}
}

func deepCopyRowData(rows []*Row) []*Row {
newRows := make([]*Row, 0, len(rows))
for _, r := range rows {
newRows = append(newRows, &Row{
Data: r.Data.clone(),
Tags: r.Tags,
})
}
return newRows
}

0 comments on commit f427a1a

Please sign in to comment.