feat: Improves EvaluationContext thread safety (#69)

fabriziodemaria · web-flow · commit 65cb2951297f · 2025-07-04T13:29:40.000+02:00
&lt;!-- Please use this template for your pull request. --&gt;
&lt;!-- Please use the sections that you need and delete other sections --&gt;

## This PR
Creates a deep copy of the evaluation context before passing it into the
provider. While the original evaluation context class could be mutated
by the hosting application, the copy will not be affected, ensuring
higher thread safety guarantees for the consuming provider.
Mutable classes are made thread safe as well.

### Related Issues
Race conditions within `onContextSet` implementation have been reported
for some Swift Providers.

### How to test
Unit tests provided for the deep copy, but difficult to test the race
condition possibility.

---------

Signed-off-by: Fabrizio Demaria &lt;fabrizio.f.demaria@gmail.com&gt;
diff --git a/Package.swift b/Package.swift
@@ -14,7 +14,7 @@ let package = Package(
     products: [
         .library(
             name: "OpenFeature",
-            targets: ["OpenFeature"])
+            targets: ["OpenFeature"]),
     ],
     dependencies: [],
     targets: [
diff --git a/Sources/OpenFeature/EvaluationContext.swift b/Sources/OpenFeature/EvaluationContext.swift
@@ -3,6 +3,6 @@ import Foundation
 /// Container for arbitrary contextual data that can be used as a basis for dynamic evaluation.
 public protocol EvaluationContext: Structure {
     func getTargetingKey() -> String
-
+    func deepCopy() -> EvaluationContext
     func setTargetingKey(targetingKey: String)
 }
diff --git a/Sources/OpenFeature/MutableContext.swift b/Sources/OpenFeature/MutableContext.swift
@@ -1,8 +1,9 @@
 import Foundation
 
-/// The ``MutableContext`` is an ``EvaluationContext`` implementation which is not threadsafe, and whose attributes can
+/// The ``MutableContext`` is an ``EvaluationContext`` implementation which is threadsafe, and whose attributes can
 /// be modified after instantiation.
 public class MutableContext: EvaluationContext {
+    private let queue = DispatchQueue(label: "com.openfeature.mutablecontext.queue", qos: .userInitiated)
     private var targetingKey: String
     private var structure: MutableStructure
 
@@ -15,35 +16,55 @@ public class MutableContext: EvaluationContext {
         self.init(structure: MutableStructure(attributes: attributes))
     }
 
+    public func deepCopy() -> EvaluationContext {
+        return queue.sync {
+            MutableContext(targetingKey: targetingKey, structure: structure.deepCopy())
+        }
+    }
+
     public func getTargetingKey() -> String {
-        return self.targetingKey
+        return queue.sync {
+            self.targetingKey
+        }
     }
 
     public func setTargetingKey(targetingKey: String) {
-        self.targetingKey = targetingKey
+        queue.sync {
+            self.targetingKey = targetingKey
+        }
     }
 
     public func keySet() -> Set<String> {
-        return structure.keySet()
+        return queue.sync {
+            structure.keySet()
+        }
     }
 
     public func getValue(key: String) -> Value? {
-        return structure.getValue(key: key)
+        return queue.sync {
+            structure.getValue(key: key)
+        }
     }
 
     public func asMap() -> [String: Value] {
-        return structure.asMap()
+        return queue.sync {
+            structure.asMap()
+        }
     }
 
     public func asObjectMap() -> [String: AnyHashable?] {
-        return structure.asObjectMap()
+        return queue.sync {
+            structure.asObjectMap()
+        }
     }
 }
 
 extension MutableContext {
     @discardableResult
     public func add(key: String, value: Value) -> MutableContext {
-        self.structure.add(key: key, value: value)
+        queue.sync {
+            self.structure.add(key: key, value: value)
+        }
         return self
     }
 }
diff --git a/Sources/OpenFeature/MutableStructure.swift b/Sources/OpenFeature/MutableStructure.swift
@@ -1,28 +1,43 @@
 import Foundation
 
-/// The ``MutableStructure`` is a ``Structure`` implementation which is not threadsafe, and whose attributes can
+/// The ``MutableStructure`` is a ``Structure`` implementation which is threadsafe, and whose attributes can
 /// be modified after instantiation.
 public class MutableStructure: Structure {
+    private let queue = DispatchQueue(label: "com.openfeature.mutablestructure.queue", qos: .userInitiated)
     private var attributes: [String: Value]
 
     public init(attributes: [String: Value] = [:]) {
         self.attributes = attributes
     }
 
     public func keySet() -> Set<String> {
-        return Set(attributes.keys)
+        return queue.sync {
+            Set(attributes.keys)
+        }
     }
 
     public func getValue(key: String) -> Value? {
-        return attributes[key]
+        return queue.sync {
+            attributes[key]
+        }
     }
 
     public func asMap() -> [String: Value] {
-        return attributes
+        return queue.sync {
+            attributes
+        }
     }
 
     public func asObjectMap() -> [String: AnyHashable?] {
-        return attributes.mapValues(convertValue)
+        return queue.sync {
+            attributes.mapValues(convertValue)
+        }
+    }
+
+    public func deepCopy() -> MutableStructure {
+        return queue.sync {
+            MutableStructure(attributes: attributes)
+        }
     }
 }
 
@@ -58,7 +73,9 @@ extension MutableStructure {
 extension MutableStructure {
     @discardableResult
     public func add(key: String, value: Value) -> MutableStructure {
-        attributes[key] = value
+        queue.sync {
+            attributes[key] = value
+        }
         return self
     }
 }
diff --git a/Sources/OpenFeature/OpenFeatureAPI.swift b/Sources/OpenFeature/OpenFeatureAPI.swift
@@ -156,11 +156,11 @@ public class OpenFeatureAPI {
         self.providerSubject.send(provider)
 
         if let initialContext = initialContext {
-            self.evaluationContext = initialContext
+            self.evaluationContext = initialContext.deepCopy()
         }
 
         do {
-            try await provider.initialize(initialContext: initialContext)
+            try await provider.initialize(initialContext: initialContext?.deepCopy())
             self.providerStatus = .ready
             self.eventHandler.send(.ready)
         } catch {
@@ -177,11 +177,14 @@ public class OpenFeatureAPI {
 
     private func updateContext(evaluationContext: EvaluationContext) async {
         do {
-            let oldContext = self.evaluationContext
-            self.evaluationContext = evaluationContext
+            let oldContext = self.evaluationContext?.deepCopy()
+            self.evaluationContext = evaluationContext.deepCopy()
             self.providerStatus = .reconciling
             eventHandler.send(.reconciling)
-            try await self.providerSubject.value?.onContextSet(oldContext: oldContext, newContext: evaluationContext)
+            try await self.providerSubject.value?.onContextSet(
+                oldContext: oldContext,
+                newContext: evaluationContext.deepCopy()
+            )
             self.providerStatus = .ready
             eventHandler.send(.contextChanged)
         } catch {
diff --git a/Sources/OpenFeature/Value.swift b/Sources/OpenFeature/Value.swift
@@ -3,6 +3,7 @@ import Foundation
 /// Values serve as a generic return type for structure data from providers.
 /// Providers may deal in JSON, protobuf, XML or some other data-interchange format.
 /// This intermediate representation provides a good medium of exchange.
+/// All the enum types must be value types, as Value is expected to support deep copies.
 public enum Value: Equatable, Codable {
     case boolean(Bool)
     case string(String)
diff --git a/Tests/OpenFeatureTests/EvalContextTests.swift b/Tests/OpenFeatureTests/EvalContextTests.swift
@@ -136,4 +136,133 @@ final class EvalContextTests: XCTestCase {
 
         XCTAssertEqual(ctx.asObjectMap(), expected)
     }
+
+    func testContextDeepCopyCreatesIndependentCopy() {
+        // Create original context with various data types
+        let originalContext = MutableContext(targetingKey: "original-key")
+        originalContext.add(key: "string", value: .string("original-value"))
+        originalContext.add(key: "integer", value: .integer(42))
+        originalContext.add(key: "boolean", value: .boolean(true))
+        originalContext.add(key: "list", value: .list([.string("item1"), .integer(100)]))
+        originalContext.add(key: "structure", value: .structure([
+            "nested-string": .string("nested-value"),
+            "nested-int": .integer(200),
+        ]))
+
+        guard let copiedContext = originalContext.deepCopy() as? MutableContext else {
+            XCTFail("Failed to cast to MutableContext")
+            return
+        }
+
+        XCTAssertEqual(copiedContext.getTargetingKey(), "original-key")
+        XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "original-value")
+        XCTAssertEqual(copiedContext.getValue(key: "integer")?.asInteger(), 42)
+        XCTAssertEqual(copiedContext.getValue(key: "boolean")?.asBoolean(), true)
+        XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[0].asString(), "item1")
+        XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[1].asInteger(), 100)
+        XCTAssertEqual(
+            copiedContext.getValue(key: "structure")?.asStructure()?["nested-string"]?.asString(),
+            "nested-value"
+        )
+        XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["nested-int"]?.asInteger(), 200)
+
+        originalContext.setTargetingKey(targetingKey: "modified-key")
+        originalContext.add(key: "string", value: .string("modified-value"))
+        originalContext.add(key: "new-key", value: .string("new-value"))
+
+        XCTAssertEqual(copiedContext.getTargetingKey(), "original-key")
+        XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "original-value")
+        XCTAssertNil(copiedContext.getValue(key: "new-key"))
+        XCTAssertEqual(originalContext.getTargetingKey(), "modified-key")
+        XCTAssertEqual(originalContext.getValue(key: "string")?.asString(), "modified-value")
+        XCTAssertEqual(originalContext.getValue(key: "new-key")?.asString(), "new-value")
+    }
+
+    func testContextDeepCopyWithEmptyContext() {
+        let emptyContext = MutableContext()
+        guard let copiedContext = emptyContext.deepCopy() as? MutableContext else {
+            XCTFail("Failed to cast to MutableContext")
+            return
+        }
+
+        XCTAssertEqual(emptyContext.getTargetingKey(), "")
+        XCTAssertEqual(copiedContext.getTargetingKey(), "")
+        XCTAssertTrue(emptyContext.keySet().isEmpty)
+        XCTAssertTrue(copiedContext.keySet().isEmpty)
+
+        emptyContext.setTargetingKey(targetingKey: "test")
+        emptyContext.add(key: "key", value: .string("value"))
+
+        XCTAssertEqual(copiedContext.getTargetingKey(), "")
+        XCTAssertTrue(copiedContext.keySet().isEmpty)
+    }
+
+    func testContextDeepCopyPreservesAllValueTypes() {
+        let date = Date()
+        let originalContext = MutableContext(targetingKey: "test-key")
+        originalContext.add(key: "null", value: .null)
+        originalContext.add(key: "string", value: .string("test-string"))
+        originalContext.add(key: "boolean", value: .boolean(false))
+        originalContext.add(key: "integer", value: .integer(12345))
+        originalContext.add(key: "double", value: .double(3.14159))
+        originalContext.add(key: "date", value: .date(date))
+        originalContext.add(key: "list", value: .list([.string("list-item"), .integer(999)]))
+        originalContext.add(key: "structure", value: .structure([
+            "struct-key": .string("struct-value"),
+            "struct-number": .integer(777),
+        ]))
+
+        guard let copiedContext = originalContext.deepCopy() as? MutableContext else {
+            XCTFail("Failed to cast to MutableContext")
+            return
+        }
+
+        XCTAssertTrue(copiedContext.getValue(key: "null")?.isNull() ?? false)
+        XCTAssertEqual(copiedContext.getValue(key: "string")?.asString(), "test-string")
+        XCTAssertEqual(copiedContext.getValue(key: "boolean")?.asBoolean(), false)
+        XCTAssertEqual(copiedContext.getValue(key: "integer")?.asInteger(), 12345)
+        XCTAssertEqual(copiedContext.getValue(key: "double")?.asDouble(), 3.14159)
+        XCTAssertEqual(copiedContext.getValue(key: "date")?.asDate(), date)
+        XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[0].asString(), "list-item")
+        XCTAssertEqual(copiedContext.getValue(key: "list")?.asList()?[1].asInteger(), 999)
+        XCTAssertEqual(
+            copiedContext.getValue(key: "structure")?.asStructure()?["struct-key"]?.asString(),
+            "struct-value"
+        )
+        XCTAssertEqual(copiedContext.getValue(key: "structure")?.asStructure()?["struct-number"]?.asInteger(), 777)
+    }
+
+    func testContextDeepCopyIsThreadSafe() {
+        let context = MutableContext(targetingKey: "initial-key")
+        context.add(key: "initial", value: .string("initial-value"))
+
+        let expectation = XCTestExpectation(description: "Concurrent deep copy operations")
+        let concurrentQueue = DispatchQueue(label: "test.concurrent", attributes: .concurrent)
+        let group = DispatchGroup()
+
+        // Perform multiple concurrent operations
+        for i in 0..<100 {
+            group.enter()
+            concurrentQueue.async {
+                // Modify the context
+                context.setTargetingKey(targetingKey: "modified-\(i)")
+                context.add(key: "key-\(i)", value: .integer(Int64(i)))
+
+                // Perform deep copy
+                let copiedContext = context.deepCopy()
+
+                // Verify the copy is independent
+                XCTAssertNotEqual(copiedContext.getTargetingKey(), "initial-key")
+                XCTAssertNotNil(copiedContext.getValue(key: "initial"))
+
+                group.leave()
+            }
+        }
+
+        group.notify(queue: .main) {
+            expectation.fulfill()
+        }
+
+        wait(for: [expectation], timeout: 5.0)
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,6 @@ import Foundation`
`3`	`3`	`/// Container for arbitrary contextual data that can be used as a basis for dynamic evaluation.`
`4`	`4`	`public protocol EvaluationContext: Structure {`
`5`	`5`	`func getTargetingKey() -> String`
`6`		`-`
	`6`	`+ func deepCopy() -> EvaluationContext`
`7`	`7`	`func setTargetingKey(targetingKey: String)`
`8`	`8`	`}`