Skip to content

Commit

Permalink
sqlstats: fix race condition in ss_mem_iterator
Browse files Browse the repository at this point in the history
StmtStatsIterator.Next() copies the next statement stats info into a
new CollectedStatementStatistics object, but the slice fields
of stmtStats.mu.data are not copied explicitly. This makes them
susecptible to race conditions, as seen in #138224.

To fix, all slice fields in the StatementStatistics are
explicitly copied, using slices.Clone.

Fixes: 138224
Epic: none
Release note: none
  • Loading branch information
kyle-a-wong committed Jan 30, 2025
1 parent 90db048 commit 3ea3deb
Showing 1 changed file with 16 additions and 1 deletion.
17 changes: 16 additions & 1 deletion pkg/sql/sqlstats/ssmemstorage/ss_mem_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package ssmemstorage

import (
"slices"
"sort"

"github.com/cockroachdb/cockroach/pkg/sql/appstatspb"
Expand Down Expand Up @@ -77,7 +78,7 @@ func (s *StmtStatsIterator) Next() bool {
}

statementStats.mu.Lock()
data := statementStats.mu.data
data := copyDataLocked(statementStats)
distSQLUsed := statementStats.mu.distSQLUsed
vectorized := statementStats.mu.vectorized
fullScan := statementStats.mu.fullScan
Expand Down Expand Up @@ -105,6 +106,20 @@ func (s *StmtStatsIterator) Next() bool {
return true
}

// copyDataLocked Copies the statement stat's data object under the mutex.
// This function requires that there is a lock on the stats object.
func copyDataLocked(stats *stmtStats) appstatspb.StatementStatistics {
stats.mu.AssertHeld()
data := stats.mu.data
data.Nodes = slices.Clone(data.Nodes)
data.KVNodeIDs = slices.Clone(data.KVNodeIDs)
data.Regions = slices.Clone(data.Regions)
data.PlanGists = slices.Clone(data.PlanGists)
data.IndexRecommendations = slices.Clone(data.IndexRecommendations)
data.Indexes = slices.Clone(data.Indexes)
return data
}

// Cur returns the appstatspb.CollectedStatementStatistics at the current internal
// counter.
func (s *StmtStatsIterator) Cur() *appstatspb.CollectedStatementStatistics {
Expand Down

0 comments on commit 3ea3deb

Please sign in to comment.