Skip to content

Commit 939e41b

Browse files
committed
Fix data race with RedisMetrics.activeConnectionCount
Motivation: After enabling the thread sanitizer during testing, a data race with the `activeConnectionCount` in `RedisMetrics` was caught due to changing a primitive `Int` across threads. Modifications: Add a specialized class `ActiveConnectionGauge` to wrap the semantics of the `activeConnectionCount` with an `Atomic<Int>` property. Result: No data races should occur with the `RedisMetrics` subsystem.
1 parent dfca570 commit 939e41b

File tree

2 files changed

+31
-11
lines changed

2 files changed

+31
-11
lines changed

Sources/RediStack/RedisConnection.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public final class RedisConnection: RedisClient {
148148
self.logger[metadataKey: loggingKeyID] = "\(UUID())"
149149
self._state = .open
150150
self.logger.debug("Connection created.")
151-
RedisMetrics.activeConnectionCount += 1
151+
RedisMetrics.activeConnectionCount.increment()
152152
RedisMetrics.totalConnectionCount.increment()
153153

154154
// attach a callback to the channel to capture situations where the channel might be closed out from under
@@ -160,7 +160,7 @@ public final class RedisConnection: RedisClient {
160160

161161
self.state = .closed
162162
self.logger.warning("Channel was closed unexpectedly.")
163-
RedisMetrics.activeConnectionCount -= 1
163+
RedisMetrics.activeConnectionCount.decrement()
164164
}
165165
}
166166

@@ -243,7 +243,7 @@ extension RedisConnection {
243243
notification.whenSuccess {
244244
self.state = .closed
245245
self.logger.debug("Connection closed.")
246-
RedisMetrics.activeConnectionCount -= 1
246+
RedisMetrics.activeConnectionCount.decrement()
247247
}
248248

249249
return notification

Sources/RediStack/RedisMetrics.swift

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
import Metrics
16+
import NIOConcurrencyHelpers
1617

1718
/// The system funnel for all `Metrics` interactions from the Redis library.
1819
///
@@ -38,14 +39,8 @@ public struct RedisMetrics {
3839
}
3940
}
4041

41-
private static let activeConnectionCountGauge = Gauge(label: .activeConnectionCount)
42-
/// The current number of connections this library has active.
43-
/// - Note: Changing this number will update the `Metrics.Gauge` stored for recording the new value.
44-
public static var activeConnectionCount: Int = 0 {
45-
didSet {
46-
activeConnectionCountGauge.record(activeConnectionCount)
47-
}
48-
}
42+
/// The wrapped `Metrics.Gauge` maintaining the current number of connections this library has active.
43+
public static var activeConnectionCount = ActiveConnectionGauge()
4944
/// The `Metrics.Counter` that retains the number of connections made since application startup.
5045
public static let totalConnectionCount = Counter(label: .totalConnectionCount)
5146
/// The `Metrics.Counter` that retains the number of commands that successfully returned from Redis
@@ -61,6 +56,31 @@ public struct RedisMetrics {
6156
private init() { }
6257
}
6358

59+
/// A specialized wrapper class for working with `Metrics.Gauge` objects for the purpose of an incrementing or decrementing count of active Redis connections.
60+
public class ActiveConnectionGauge {
61+
private let gauge = Gauge(label: .activeConnectionCount)
62+
private let count = Atomic<Int>(value: 0)
63+
64+
/// The number of the connections that are currently reported as active.
65+
var currentCount: Int { return count.load() }
66+
67+
internal init() { }
68+
69+
/// Increments the current count by the amount specified.
70+
/// - Parameter amount: The number to increase the current count by. Default is `1`.
71+
public func increment(by amount: Int = 1) {
72+
_ = self.count.add(amount)
73+
self.gauge.record(self.count.load())
74+
}
75+
76+
/// Decrements the current count by the amount specified.
77+
/// - Parameter amount: The number to decrease the current count by. Default is `1`.
78+
public func decrement(by amount: Int = 1) {
79+
_ = self.count.sub(amount)
80+
self.gauge.record(self.count.load())
81+
}
82+
}
83+
6484
extension Metrics.Counter {
6585
@inline(__always)
6686
convenience init(label: RedisMetrics.Label) {

0 commit comments

Comments
 (0)