Skip to content

Commit

Permalink
Fix deadlock in recursive metric locks
Browse files Browse the repository at this point in the history
RecordMetrics() grabs a mutex and calls recordCancelMetric(), which wants to
grab the same mutex. Go mutexes are not recursive, so recordCancelMetric
blocks forever.

recordCancelMetric should not grab the mutex, it can be sure that the
caller did it already.
  • Loading branch information
jsafrane authored and xing-yang committed Aug 24, 2021
1 parent f6026ab commit 3c53046
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions pkg/metrics/metrics.go
Expand Up @@ -231,15 +231,15 @@ func (opMgr *operationMetricsManager) RecordMetrics(opKey OperationKey, opStatus
obj, exists := opMgr.cache[createKey]
if exists {
// record a cancel metric if found
opMgr.recordCancelMetric(obj, createKey, operationDuration)
opMgr.recordCancelMetricLocked(obj, createKey, operationDuration)
}

// check if we have a CreateSnapshotAndReady operation pending for this
createAndReadyKey := NewOperationKey(CreateSnapshotAndReadyOperationName, opKey.ResourceID)
obj, exists = opMgr.cache[createAndReadyKey]
if exists {
// record a cancel metric if found
opMgr.recordCancelMetric(obj, createAndReadyKey, operationDuration)
opMgr.recordCancelMetricLocked(obj, createAndReadyKey, operationDuration)
}
}

Expand All @@ -248,9 +248,8 @@ func (opMgr *operationMetricsManager) RecordMetrics(opKey OperationKey, opStatus
}

// recordCancelMetric records a metric for a create operation that hasn't finished
func (opMgr *operationMetricsManager) recordCancelMetric(val OperationValue, key OperationKey, duration float64) {
opMgr.mu.Lock()
defer opMgr.mu.Unlock()
// This function must be called with opMgr mutex locked (to prevent recursive locks).
func (opMgr *operationMetricsManager) recordCancelMetricLocked(val OperationValue, key OperationKey, duration float64) {
// record a cancel metric if found

opMgr.opLatencyMetrics.WithLabelValues(
Expand Down

0 comments on commit 3c53046

Please sign in to comment.