Skip to content

Commit d4097d4

Browse files
authored
Merge pull request #35 from instana/feature/improve_thread_safety
Add new atomic types for some collections
2 parents 1b94e54 + 45facfa commit d4097d4

19 files changed

+245
-75
lines changed

Sources/InstanaAgent/Configuration/InstanaConfiguraton.swift

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import Foundation
1515
case none
1616
}
1717

18-
struct InstanaConfiguration: Equatable {
18+
// Use a reference type to avoid a copy when having concurrency
19+
class InstanaConfiguration {
1920
enum SuspendReporting {
2021
/// Reporting is suspended while the device battery is low.
2122
case lowBattery
@@ -63,7 +64,7 @@ struct InstanaConfiguration: Equatable {
6364
var reporterSendLowBatteryDebounce: Instana.Types.Seconds
6465
var maxRetries: Int
6566
var gzipReport: Bool
66-
var maxBeaconsPerRequest: Int
67+
var maxBeaconsPerRequest: Int = 0
6768
var maxQueueSize: Int
6869
var preQueueUsageTime: TimeInterval
6970
var reporterRateLimits: [ReporterRateLimitConfig]
@@ -73,19 +74,23 @@ struct InstanaConfiguration: Equatable {
7374
.default(key: "", reportingURL: URL(string: "https://www.instana.com")!, httpCaptureConfig: .none)
7475
}
7576

77+
required init(reportingURL: URL, key: String, httpCaptureConfig: HTTPCaptureConfig) {
78+
self.reportingURL = reportingURL
79+
self.key = key
80+
self.httpCaptureConfig = httpCaptureConfig
81+
suspendReporting = SuspendReporting.defaults
82+
monitorTypes = MonitorTypes.current
83+
reporterSendDebounce = Defaults.reporterSendDebounce
84+
reporterSendLowBatteryDebounce = Defaults.reporterSendLowBatteryDebounce
85+
maxRetries = Defaults.maxRetries
86+
gzipReport = Defaults.gzipReport
87+
maxBeaconsPerRequest = Defaults.maxBeaconsPerRequest
88+
maxQueueSize = Defaults.maxQueueSize
89+
preQueueUsageTime = Defaults.preQueueUsageTime
90+
reporterRateLimits = Defaults.reporterRateLimits
91+
}
92+
7693
static func `default`(key: String, reportingURL: URL, httpCaptureConfig: HTTPCaptureConfig = .automatic) -> InstanaConfiguration {
77-
self.init(reportingURL: reportingURL,
78-
key: key,
79-
httpCaptureConfig: httpCaptureConfig,
80-
suspendReporting: SuspendReporting.defaults,
81-
monitorTypes: MonitorTypes.current,
82-
reporterSendDebounce: Defaults.reporterSendDebounce,
83-
reporterSendLowBatteryDebounce: Defaults.reporterSendLowBatteryDebounce,
84-
maxRetries: Defaults.maxRetries,
85-
gzipReport: Defaults.gzipReport,
86-
maxBeaconsPerRequest: Defaults.maxBeaconsPerRequest,
87-
maxQueueSize: Defaults.maxQueueSize,
88-
preQueueUsageTime: Defaults.preQueueUsageTime,
89-
reporterRateLimits: Defaults.reporterRateLimits)
94+
self.init(reportingURL: reportingURL, key: key, httpCaptureConfig: httpCaptureConfig)
9095
}
9196
}

Sources/InstanaAgent/Configuration/InstanaProperties.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import Foundation
77
typealias MetaData = [String: String]
88
typealias HTTPHeader = [String: String]
99

10-
struct InstanaProperties: Equatable {
10+
// Make this a reference type to avoid a copy for concurrency
11+
class InstanaProperties {
1112
struct User: Equatable {
1213
static let userValueMaxLength = 128
1314

@@ -51,7 +52,7 @@ struct InstanaProperties: Equatable {
5152
self.view = Self.validate(view: view)
5253
}
5354

54-
mutating func appendMetaData(_ key: String, _ value: String) {
55+
func appendMetaData(_ key: String, _ value: String) {
5556
let key = MetaData.validate(key: key)
5657
let value = MetaData.validate(value: value)
5758
if metaData.count < MetaData.Max.numberOfMetaEntries {

Sources/InstanaAgent/Instana.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,9 @@ import Foundation
378378
/// Default: No HTTP header fields are captured. Keywords must be provided explicitly
379379
///
380380
/// - Parameters:
381-
/// - regex: An array of NSRegularExpression objects to match the key of HTTP request/response headers that you want to capture.
381+
/// - regex: An array of NSRegularExpression objects to match the key of HTTP request/response headers that you want to capture.
382382
@objc
383383
public static func setCaptureHeaders(matching regex: [NSRegularExpression]) {
384-
Instana.current?.monitors.http?.filter.headerFieldsRegEx = regex
384+
Instana.current?.monitors.http?.filter.headerFieldsRegEx = AtomicArray(regex)
385385
}
386386
}

Sources/InstanaAgent/Monitors/HTTP/HTTPMonitorFilter.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import Foundation
22

33
class HTTPMonitorFilter {
44
private let redactionHandler: RedactionHandler
5-
@Atomic var headerFieldsRegEx = [NSRegularExpression]()
5+
var headerFieldsRegEx = AtomicArray<NSRegularExpression>()
66

77
init(redactionHandler: RedactionHandler = .default) {
88
self.redactionHandler = redactionHandler
@@ -13,7 +13,7 @@ class HTTPMonitorFilter {
1313
}
1414

1515
func setRedaction(regex: [NSRegularExpression]) {
16-
redactionHandler.regex = Set(regex)
16+
redactionHandler.regex = AtomicSet(regex)
1717
}
1818

1919
func filterHeaderFields(_ header: HTTPHeader?) -> HTTPHeader? {

Sources/InstanaAgent/Monitors/HTTP/IgnoreURLHandler.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import Foundation
66

77
struct IgnoreURLHandler {
88
/// Monitor ignores URLs that match the given regular expressions
9-
@Atomic static var regex = Set<NSRegularExpression>()
9+
static var regex = AtomicSet<NSRegularExpression>()
1010

1111
/// Monitor ignores the exact URLs given in this collection
12-
@Atomic static var exactURLs = Set<URL>()
12+
static var exactURLs = AtomicSet<URL>()
1313

1414
/// All sessions will be ignored from HTTP monitoring
15-
@Atomic static var urlSessions = Set<URLSession>()
15+
static var urlSessions = AtomicSet<URLSession>()
1616

1717
static func ignore(session: URLSession) {
1818
urlSessions.insert(session)

Sources/InstanaAgent/Monitors/HTTP/InstanaURLProtocol.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ extension URLSessionConfiguration {
355355
}
356356
}
357357

358-
@Atomic static var all = [URLSessionConfiguration]()
358+
static var all = AtomicArray<URLSessionConfiguration>()
359359

360360
static func removeAllInstanaURLProtocol() {
361361
all.forEach { $0.protocolClasses?.removeAll(where: { (protocolClass) -> Bool in

Sources/InstanaAgent/Monitors/HTTP/RedactionHandler.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ class RedactionHandler {
99
return RedactionHandler(regex: regex)
1010
}
1111

12-
@Atomic var regex = Set<NSRegularExpression>()
12+
var regex = AtomicSet<NSRegularExpression>()
1313

1414
init(regex: [NSRegularExpression]) {
15-
self.regex = Set(regex)
15+
self.regex = AtomicSet(regex)
1616
}
1717

1818
func redact(url: URL) -> URL {

Sources/InstanaAgent/Utils/InstanaApplicationStateHandler.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class InstanaApplicationStateHandler {
2626
}
2727

2828
typealias StateUpdater = (State) -> Void
29-
private var stateUpdateHandler = [StateUpdater]()
29+
private var stateUpdateHandler = AtomicArray<StateUpdater>()
3030

3131
init() {
3232
_ = NotificationCenter.default.addObserver(forName: Application.didBecomeActiveNotification, object: nil, queue: nil) { _ in

Sources/InstanaAgent/Utils/PropertyWrapper/Atomic.swift

Lines changed: 145 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import Foundation
66

7+
/// Don't use this propertyWrapper for value types, esp. Collections (Array, Dictionary, Set)
8+
/// This can have unexpected results: see https://www.donnywals.com/why-your-atomic-property-wrapper-doesnt-work-for-collection-types/
79
@propertyWrapper
810
struct Atomic<Value> {
911
private var value: Value
@@ -14,19 +16,156 @@ struct Atomic<Value> {
1416
}
1517

1618
var wrappedValue: Value {
17-
get { return load() }
18-
set { store(newValue: newValue) }
19+
get { lock.atomic { value } }
20+
set { lock.atomic { value = newValue } }
21+
}
22+
}
23+
24+
extension NSLock {
25+
func atomic<T>(_ closure: () throws -> T) rethrows -> T {
26+
lock()
27+
defer {
28+
unlock()
29+
}
30+
return try closure()
31+
}
32+
}
33+
34+
// Inspired by donnywals https://www.donnywals.com/why-your-atomic-property-wrapper-doesnt-work-for-collection-types/
35+
// We must make this a reference type to avoid getting a copy for each different thread (expected value type behavior)
36+
class AtomicDictionary<Key: Hashable, Value>: CustomDebugStringConvertible {
37+
private var dict = [Key: Value]()
38+
private let lock = NSLock()
39+
40+
subscript(key: Key) -> Value? {
41+
get { lock.atomic { dict[key] } }
42+
set { lock.atomic { dict[key] = newValue } }
43+
}
44+
45+
var debugDescription: String {
46+
lock.atomic { dict.debugDescription }
47+
}
48+
}
49+
50+
extension AtomicDictionary where Value: Equatable {
51+
static func == <Key: Hashable, Value: Equatable>(lhs: AtomicDictionary<Key, Value>, rhs: AtomicDictionary<Key, Value>) -> Bool {
52+
lhs.dict == rhs.dict
53+
}
54+
}
55+
56+
class AtomicArray<T>: CustomDebugStringConvertible, Sequence, Collection {
57+
private var array: [T]
58+
private let lock = NSLock()
59+
60+
var startIndex: Int {
61+
lock.atomic { array.startIndex }
62+
}
63+
64+
var endIndex: Int {
65+
lock.atomic { array.endIndex }
66+
}
67+
68+
init(_ array: [T] = []) {
69+
self.array = array
70+
}
71+
72+
subscript(index: Int) -> T {
73+
get { lock.atomic { array[index] } }
74+
set { lock.atomic { array[index] = newValue } }
75+
}
76+
77+
func append(_ newElement: T) {
78+
lock.atomic { array.append(newElement) }
79+
}
80+
81+
var debugDescription: String {
82+
lock.atomic { array.debugDescription }
83+
}
84+
85+
func removeAll() {
86+
lock.atomic { array.removeAll() }
87+
}
88+
89+
func removeAll(where shouldBeRemoved: (Element) throws -> Bool) rethrows {
90+
try lock.atomic {
91+
try array.removeAll(where: shouldBeRemoved)
92+
}
1993
}
2094

21-
private func load() -> Value {
95+
func index(after index: Int) -> Int {
96+
lock.atomic {
97+
array.index(after: index)
98+
}
99+
}
100+
101+
func index(before index: Int) -> Int {
102+
lock.atomic {
103+
array.index(before: index)
104+
}
105+
}
106+
107+
func makeIterator() -> IndexingIterator<[T]> {
22108
lock.lock()
23109
defer { lock.unlock() }
24-
return value
110+
return array.makeIterator()
111+
}
112+
}
113+
114+
extension AtomicArray where T: Equatable {
115+
func contains(_ element: T) -> Bool {
116+
lock.atomic { array.contains(element) }
117+
}
118+
119+
static func == <T: Equatable>(lhs: AtomicArray<T>, rhs: AtomicArray<T>) -> Bool {
120+
lhs.array == rhs.array
121+
}
122+
}
123+
124+
class AtomicSet<T: Hashable>: CustomDebugStringConvertible, Sequence {
125+
private var set: Set<T>
126+
private let lock = NSLock()
127+
128+
init(_ set: [T] = []) {
129+
self.set = Set(set)
130+
}
131+
132+
func contains(_ member: T) -> Bool {
133+
lock.atomic { set.contains(member) }
25134
}
26135

27-
private mutating func store(newValue: Value) {
136+
@discardableResult func insert(_ newMember: T) -> (inserted: Bool, memberAfterInsert: T) {
137+
lock.atomic { set.insert(newMember) }
138+
}
139+
140+
var debugDescription: String {
141+
lock.atomic { set.debugDescription }
142+
}
143+
144+
func makeIterator() -> Set<T>.Iterator {
28145
lock.lock()
29146
defer { lock.unlock() }
30-
value = newValue
147+
return set.makeIterator()
148+
}
149+
150+
func formUnion<S>(_ other: S) where T == S.Element, S: Sequence {
151+
lock.atomic { set.formUnion(other) }
152+
}
153+
154+
func removeAll() {
155+
lock.atomic { set.removeAll() }
156+
}
157+
158+
var count: Int {
159+
lock.atomic { set.count }
160+
}
161+
162+
var first: T? {
163+
lock.atomic { set.first }
164+
}
165+
}
166+
167+
extension AtomicSet: Equatable {
168+
static func == <T>(lhs: AtomicSet<T>, rhs: AtomicSet<T>) -> Bool {
169+
lhs.set == rhs.set
31170
}
32171
}

Tests/InstanaAgentTests/Beacons/ReporterTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,7 +1174,7 @@ extension ReporterTests {
11741174
extension ReporterTests {
11751175

11761176
func createMockSession(_ delay: Instana.Types.Seconds = 0.0, preQueueUsageTime: Instana.Types.Seconds = 0.0, suspend: Set<InstanaConfiguration.SuspendReporting> = [], gzip: Bool = false) -> InstanaSession {
1177-
var config = InstanaConfiguration.mock(gzipReport: gzip)
1177+
let config = InstanaConfiguration.mock(gzipReport: gzip)
11781178
config.reporterSendDebounce = delay
11791179
config.reporterSendLowBatteryDebounce = delay
11801180
config.preQueueUsageTime = preQueueUsageTime
@@ -1208,7 +1208,7 @@ extension ReporterTests {
12081208
}
12091209

12101210
func mockBeaconSubmission(_ loadResult: Swift.Result<Int, Error>, resultCallback: @escaping (BeaconResult) -> Void) {
1211-
var config = InstanaConfiguration.mock
1211+
let config = InstanaConfiguration.mock
12121212
config.reporterSendDebounce = 0.0
12131213
config.reporterSendLowBatteryDebounce = 0.0
12141214
let reporter = Reporter(.mock(configuration: config),

0 commit comments

Comments
 (0)