diff --git a/Sources/SwiftEnvironment/Environment/GlobalEnvironment.swift b/Sources/SwiftEnvironment/Environment/GlobalEnvironment.swift index 16448c0..a321932 100644 --- a/Sources/SwiftEnvironment/Environment/GlobalEnvironment.swift +++ b/Sources/SwiftEnvironment/Environment/GlobalEnvironment.swift @@ -79,8 +79,8 @@ public final class GlobalEnvironment: DynamicProperty, PropertyWrapperDis .weakAssign(to: \.resolvedValue, on: self) .store(in: &cancellables) - GlobalValues.assignedResolversSubject - .map { $0.1.id } + GlobalValues.currentAssignmentsPublisher + .map { $0.resolver.id } .removeDuplicates() .ensureOnMain() .weakAssign(to: \.lastAssignmentId, on: self) @@ -94,8 +94,12 @@ public final class GlobalEnvironment: DynamicProperty, PropertyWrapperDis private func atomicWrite(onMain: Bool = false, _ block: () throws -> Result) rethrows -> Result { return try atomicRun(flags: .barrier, onMain: onMain, block) } - - private func atomicRun(flags: DispatchWorkItemFlags = [], onMain: Bool = false, _ block: () throws -> Result) rethrows -> Result { + + private func atomicRun( + flags: DispatchWorkItemFlags = [], + onMain: Bool = false, + _ block: () throws -> Result + ) rethrows -> Result { guard onMain else { return try accessQueue.sync(flags: flags, execute: block) } diff --git a/Sources/SwiftEnvironment/Environment/GlobalValues.swift b/Sources/SwiftEnvironment/Environment/GlobalValues.swift index fb68286..d8d83a9 100644 --- a/Sources/SwiftEnvironment/Environment/GlobalValues.swift +++ b/Sources/SwiftEnvironment/Environment/GlobalValues.swift @@ -8,6 +8,12 @@ import Foundation import Combine +struct GlobalValuesAssignment { + let keyPath: PartialKeyPath + let resolver: InstanceResolver + let revision: UInt64 +} + /// A struct that provides global access to environment values using dynamic member lookup. /// /// `GlobalValues` is the central registry for all global environment values in your application. @@ -26,16 +32,28 @@ import Combine @dynamicMemberLookup public struct GlobalValues: @unchecked Sendable { - private static let accessQueue = DispatchQueueExecutor( - DispatchQueue(label: "GlobalValues.accessQueue", attributes: .concurrent) - ) - /// A dictionary mapping key paths to their instance resolvers. private static var underlyingResolvers: [PartialKeyPath: InstanceResolver] = [:] - + + /// The latest assignment revision for each key path. + private static var assignmentRevisions: [PartialKeyPath: UInt64] = [:] + + /// Serializes access to the global environment registry. + private static let accessLock = NSRecursiveLock() + /// A subject that emits when resolvers are assigned to key paths. /// This can be used to observe changes to global values. - private(set) static var assignedResolversSubject: PassthroughSubject<(PartialKeyPath, InstanceResolver), Never> = .init() + private(set) static var assignedResolversSubject: PassthroughSubject = .init() + + static var currentAssignmentsPublisher: AnyPublisher { + assignedResolversSubject + .filter { assignment in + atomicRead { + assignmentRevisions[assignment.keyPath] == assignment.revision + } + } + .eraseToAnyPublisher() + } /// Resets all global values by clearing resolvers and creating a new subject. /// @@ -43,6 +61,7 @@ public struct GlobalValues: @unchecked Sendable { static func reset() { GlobalValues.atomicWrite { GlobalValues.underlyingResolvers.removeAll() + GlobalValues.assignmentRevisions.removeAll() GlobalValues.assignedResolversSubject = .init() } } @@ -52,9 +71,10 @@ public struct GlobalValues: @unchecked Sendable { /// - Parameter key: The key path to the value. /// - Returns: The resolved value, or `nil` if no resolver is registered. public subscript(_ key: KeyPath) -> Value? { - GlobalValues.atomicRead { - return GlobalValues.underlyingResolvers[key]?.resolve(for: Value.self) + let resolver = GlobalValues.atomicRead { + GlobalValues.underlyingResolvers[key] } + return resolver?.resolve(for: Value.self) } /// Gets the value for a given key path using dynamic member lookup. @@ -70,9 +90,9 @@ public struct GlobalValues: @unchecked Sendable { /// - Parameter keyPath: The key path to observe. /// - Returns: A publisher that emits the current value and any future updates. static func publisher(for keyPath: KeyPath) -> AnyPublisher { - assignedResolversSubject - .filter { $0.0 == keyPath } - .compactMap { $0.1.resolve(for: Value.self) } + currentAssignmentsPublisher + .filter { $0.keyPath == keyPath } + .compactMap { $0.resolver.resolve(for: Value.self) } .eraseToAnyPublisher() } @@ -164,28 +184,34 @@ public struct GlobalValues: @unchecked Sendable { /// - resolver: The resolver to assign. /// - keyPath: The key path to assign the resolver to. private static func assign(resolver: InstanceResolver, to keyPath: PartialKeyPath) { - atomicWrite { + let result: (subject: PassthroughSubject, assignment: GlobalValuesAssignment) = atomicWrite { + let revision = GlobalValues.assignmentRevisions[keyPath, default: 0] + 1 GlobalValues.underlyingResolvers[keyPath] = resolver - GlobalValues.assignedResolversSubject.send((keyPath, resolver)) + GlobalValues.assignmentRevisions[keyPath] = revision + return ( + GlobalValues.assignedResolversSubject, + GlobalValuesAssignment(keyPath: keyPath, resolver: resolver, revision: revision) + ) } + result.subject.send(result.assignment) } - /// Reads a value atomically using a concurrent dispatch queue. + /// Reads registry state while holding its recursive lock. /// /// This ensures thread-safe access to the global values registry. /// /// - Parameter block: A closure that reads the value. /// - Returns: The result of the closure. public static func atomicRead(_ block: () throws -> Result) rethrows -> Result { - try accessQueue.sync { - try block() - } + accessLock.lock() + defer { accessLock.unlock() } + return try block() } - + private static func atomicWrite(_ block: () throws -> Result) rethrows -> Result { - try accessQueue.sync(flags: .barrier) { - try block() - } + accessLock.lock() + defer { accessLock.unlock() } + return try block() } } diff --git a/Sources/SwiftEnvironmentMacro/Macro/GlobalEntryMacro.swift b/Sources/SwiftEnvironmentMacro/Macro/GlobalEntryMacro.swift index 9d946d2..2b6e4b6 100644 --- a/Sources/SwiftEnvironmentMacro/Macro/GlobalEntryMacro.swift +++ b/Sources/SwiftEnvironmentMacro/Macro/GlobalEntryMacro.swift @@ -39,9 +39,7 @@ public struct GlobalEntryMacro: AccessorMacro, PeerMacro { AccessorDeclSyntax( """ get { - GlobalValues.atomicRead { - self[\\.\(raw: varName)] ?? GlobalValues.___\(raw: varName).value - } + self[\\.\(raw: varName)] ?? GlobalValues.___\(raw: varName).value } """ ) diff --git a/Tests/SwiftEnvironmentTests/IntegrationTests.swift b/Tests/SwiftEnvironmentTests/IntegrationTests.swift index eb75a3f..ab24dee 100644 --- a/Tests/SwiftEnvironmentTests/IntegrationTests.swift +++ b/Tests/SwiftEnvironmentTests/IntegrationTests.swift @@ -7,6 +7,7 @@ // import XCTest +import Combine @testable import SwiftEnvironment final class IntegrationTests: XCTestCase { @@ -73,9 +74,68 @@ final class IntegrationTests: XCTestCase { func test_givenConnectedInjection_whenGet_shouldReturnDefault() { GlobalValues.environment(\.dummy, DummyClass()) GlobalValues.use(\.dummy, for: \.fifthDummy) - + @GlobalEnvironment(\.fifthDummy) var dummy - + XCTAssertEqual(dummy, "dummy") } + + func test_givenActiveObserver_whenRegisteringResolverThatReadsAnotherValue_shouldNotDeadlock() { + // given: a base value, plus an active observer of the value we are about to register. + // The observer makes assignment eagerly resolve the new value during registration. + GlobalValues.environment(\.dummy, DummyClass()) + @GlobalEnvironment(\.secondDummy) var observer + + // when: the resolver reads another GlobalValues entry while being constructed, i.e. + // resolution reentrantly accesses the registry on the same thread. + let finished = expectation(description: "registration completes without deadlock") + DispatchQueue.global().async { + GlobalValues.environment(\.secondDummy) { + _ = GlobalValues.dummy + return DummyClass() + } + finished.fulfill() + } + + // then: it must not deadlock (previously a barrier write nesting a read deadlocked). + wait(for: [finished], timeout: 5) + XCTAssertNotNil(observer) + } + + func test_givenResolveQueue_whenResolverReadsAnotherValue_shouldNotDeadlock() { + GlobalValues.environment(\.dummy, DummyClass()) + GlobalValues.environment(\.secondDummy, resolveOn: DispatchQueue(label: "resolver.queue")) { + _ = GlobalValues.dummy + return DummyClass() + } + + let finished = expectation(description: "queued resolution completes without deadlock") + DispatchQueue.global().async { + _ = GlobalValues.secondDummy + finished.fulfill() + } + + wait(for: [finished], timeout: 5) + } + + func test_givenStaleAssignmentEvent_whenPublishingValue_shouldIgnoreStaleResolver() { + let newerValue = DummyClass() + let staleValue = DummyClass() + var receivedValues: [DummyDependency] = [] + let cancellable = GlobalValues.publisher(for: \.dummy) + .sink { receivedValues.append($0) } + + GlobalValues.environment(\.dummy, newerValue) + GlobalValues.assignedResolversSubject.send( + GlobalValuesAssignment( + keyPath: \.dummy, + resolver: SingletonInstanceResolver(queue: nil) { staleValue }, + revision: 0 + ) + ) + + XCTAssertEqual(receivedValues.count, 1) + XCTAssertTrue(receivedValues.first === newerValue) + withExtendedLifetime(cancellable) {} + } } diff --git a/Tests/SwiftEnvironmentTests/MacroTests.swift b/Tests/SwiftEnvironmentTests/MacroTests.swift index 8b4b913..3c2370c 100644 --- a/Tests/SwiftEnvironmentTests/MacroTests.swift +++ b/Tests/SwiftEnvironmentTests/MacroTests.swift @@ -30,9 +30,7 @@ private let basicExpansion = #""" extension GlobalValues { var dummy: DummyDependency { get { - GlobalValues.atomicRead { - self[\.dummy] ?? GlobalValues.___dummy.value - } + self[\.dummy] ?? GlobalValues.___dummy.value } }