diff --git a/Sources/Dependencies/DependencyValues.swift b/Sources/Dependencies/DependencyValues.swift index ba51b626..f1d723eb 100644 --- a/Sources/Dependencies/DependencyValues.swift +++ b/Sources/Dependencies/DependencyValues.swift @@ -291,55 +291,23 @@ public struct DependencyValues: Sendable { let cacheKey = CachedValues.CacheKey(id: TypeIdentifier(key), context: context) guard !cachedValues.cached.keys.contains(cacheKey) else { if cachedValues.cached[cacheKey]?.preparationID != DependencyValues.preparationID { - reportIssue( - { - var dependencyDescription = "" - if let fileID = DependencyValues.currentDependency.fileID, - let line = DependencyValues.currentDependency.line - { - dependencyDescription.append( - """ - Location: - \(fileID):\(line) - - """ - ) - } - dependencyDescription.append( - Key.self == Key.Value.self - ? """ - Dependency: - \(typeName(Key.Value.self)) - """ - : """ - Key: - \(typeName(Key.self)) - Value: - \(typeName(Key.Value.self)) - """ - ) - var argument: String { - "\(function)" == "subscript(key:)" - ? "\(typeName(Key.self)).self" - : "\\.\(function)" - } - return """ - @Dependency(\(argument)) has already been accessed or prepared. - - \(dependencyDescription) - - A global dependency can only be prepared a single time and cannot be accessed \ - beforehand. Prepare dependencies as early as possible in the lifecycle of your \ - application. - - To temporarily override a dependency in your application, use 'withDependencies' \ - to do so in a well-defined scope. - """ - }(), - fileID: DependencyValues.currentDependency.fileID ?? fileID, - filePath: DependencyValues.currentDependency.filePath ?? filePath, - line: DependencyValues.currentDependency.line ?? line, - column: DependencyValues.currentDependency.column ?? column + reportDependencyIssue( + message: { argument, dependencyDescription in + """ + @Dependency(\(argument)) has already been accessed or prepared. + + \(dependencyDescription) + + A global dependency can only be prepared a single time and cannot be accessed \ + beforehand. Prepare dependencies as early as possible in the lifecycle of your \ + application. + + To temporarily override a dependency in your application, use 'withDependencies' \ + to do so in a well-defined scope. + """ + }, + for: Key.self, + fileID: fileID, filePath: filePath, function: function, line: line, column: column ) } else { cachedValues.cached[cacheKey] = CachedValues.CachedValue( @@ -457,6 +425,7 @@ private let defaultContext: DependencyContext = { @_spi(Internals) public final class CachedValues: @unchecked Sendable { @TaskLocal static var isAccessingCachedDependencies = false + @TaskLocal static var isResetting = false public struct CacheKey: Hashable, Sendable { let id: TypeIdentifier @@ -486,7 +455,9 @@ public final class CachedValues: @unchecked Sendable { public func resetCache() { lock.lock() defer { lock.unlock() } - cached = [:] + Self.$isResetting.withValue(true) { + cached = [:] + } } func value( @@ -509,65 +480,54 @@ public final class CachedValues: @unchecked Sendable { !(cached[cacheKey] != nil && cached[cacheKey]?.preparationID != nil), !(key is any DependencyKey.Type) { - reportIssue( - { - var dependencyDescription = "" - if let fileID = DependencyValues.currentDependency.fileID, - let line = DependencyValues.currentDependency.line - { - dependencyDescription.append( - """ - Location: - \(fileID):\(line) - - """ - ) - } - dependencyDescription.append( - Key.self == Key.Value.self - ? """ - Dependency: - \(typeName(Key.Value.self)) - """ - : """ - Key: - \(typeName(Key.self)) - Value: - \(typeName(Key.Value.self)) - """ - ) - - var argument: String { - "\(function)" == "subscript(key:)" - ? "\(typeName(Key.self)).self" - : "\\.\(function)" - } - return """ - @Dependency(\(argument)) has no live implementation, but was accessed from a live \ - context. + reportDependencyIssue( + message: { argument, dependencyDescription in + """ + @Dependency(\(argument)) has no live implementation, but was accessed from a live \ + context. - \(dependencyDescription) + \(dependencyDescription) - To fix you can do one of two things: + To fix you can do one of two things: - • Conform '\(typeName(Key.self))' to the 'DependencyKey' protocol by providing \ - a live implementation of your dependency, and make sure that the conformance is \ - linked with this current application. + • Conform '\(typeName(Key.self))' to the 'DependencyKey' protocol by providing \ + a live implementation of your dependency, and make sure that the conformance is \ + linked with this current application. - • Override the implementation of '\(typeName(Key.self))' using \ - 'withDependencies'. This is typically done at the entry point of your \ - application, but can be done later too. - """ - }(), - fileID: DependencyValues.currentDependency.fileID ?? fileID, - filePath: DependencyValues.currentDependency.filePath ?? filePath, - line: DependencyValues.currentDependency.line ?? line, - column: DependencyValues.currentDependency.column ?? column + • Override the implementation of '\(typeName(Key.self))' using \ + 'withDependencies'. This is typically done at the entry point of your \ + application, but can be done later too. + """ + }, + for: Key.self, + fileID: fileID, filePath: filePath, function: function, line: line, column: column ) } #endif - guard let base = cached[cacheKey]?.base, let value = base as? Key.Value + #if DEBUG + let isResetting = Self.isResetting + if isResetting { + reportDependencyIssue( + message: { argument, dependencyDescription in + """ + @Dependency(\(argument)) was access during a cache reset. + + \(dependencyDescription) + + Accessing a dependency during a cache reset will always return a new and uncached \ + instance of the dependency. + """ + }, + for: Key.self, + fileID: fileID, filePath: filePath, function: function, line: line, column: column + ) + } + #else + let isResetting = false + #endif + + guard !isResetting, let base = cached[cacheKey]?.base, let value = base as? Key.Value else { let value: Key.Value? switch context { @@ -606,10 +566,12 @@ public final class CachedValues: @unchecked Sendable { } let cacheableValue = value ?? Key.testValue - cached[cacheKey] = CachedValue( - base: cacheableValue, - preparationID: DependencyValues.preparationID - ) + if !isResetting { + cached[cacheKey] = CachedValue( + base: cacheableValue, + preparationID: DependencyValues.preparationID + ) + } return cacheableValue } @@ -618,6 +580,56 @@ public final class CachedValues: @unchecked Sendable { } } +public func reportDependencyIssue( + message: (_ argument: String, _ dependencyDescription: String) -> String, + for key: Key.Type, + fileID: StaticString, + filePath: StaticString, + function: StaticString, + line: UInt, + column: UInt +) { + var dependencyDescription = "" + if let fileID = DependencyValues.currentDependency.fileID, + let line = DependencyValues.currentDependency.line + { + dependencyDescription.append( + """ + Location: + \(fileID):\(line) + + """ + ) + } + dependencyDescription.append( + Key.self == Key.Value.self + ? """ + Dependency: + \(typeName(Key.Value.self)) + """ + : """ + Key: + \(typeName(Key.self)) + Value: + \(typeName(Key.Value.self)) + """ + ) + + var argument: String { + "\(function)" == "subscript(key:)" + ? "\(typeName(Key.self)).self" + : "\\.\(function)" + } + + reportIssue( + message(argument, dependencyDescription), + fileID: DependencyValues.currentDependency.fileID ?? fileID, + filePath: DependencyValues.currentDependency.filePath ?? filePath, + line: DependencyValues.currentDependency.line ?? line, + column: DependencyValues.currentDependency.column ?? column + ) +} + struct TypeIdentifier: Hashable { let id: ObjectIdentifier #if DEBUG diff --git a/Tests/DependenciesTests/SimultaneousCacheAccessTests.swift b/Tests/DependenciesTests/SimultaneousCacheAccessTests.swift new file mode 100644 index 00000000..288f6ed4 --- /dev/null +++ b/Tests/DependenciesTests/SimultaneousCacheAccessTests.swift @@ -0,0 +1,71 @@ +@_spi(Internals) import Dependencies +import XCTest + +final class SimultaneousCacheAccessTests: XCTestCase { + // Test dependency holds onto a mock Product. Resetting the cache causes the mock Product to be + // released triggering the simultaneous access from Product's deinit. + func testDeinitCacheAccess() { + XCTExpectFailure { + @Dependency(FactoryDependency.self) var factory + _ = factory + + // Reset the cache to trigger Product deinit + DependencyValues._current.cachedValues.resetCache() + } issueMatcher: { issue in + // Accessing a value during cache reset accesses both DependencyContextKey and the accessed + // dependency. Just match on the end of the error message to cover both keys. + issue.compactDescription.hasSuffix( + "Accessing a dependency during a cache reset will always return a new and uncached instance of the dependency." + ) + } + } + + // The live dependency does not hold onto a Product, so there's no simultaneous access on reset. + func testLiveDeinit() { + withDependencies { + $0.context = .live + } operation: { + @Dependency(FactoryDependency.self) var factory + _ = factory + + // Reset the cache to validate that a Product deinit is not triggered + DependencyValues._current.cachedValues.resetCache() + } + } +} + +struct NestedDependency: TestDependencyKey { + static let testValue = NestedDependency() +} + +// Product accesses a dependency in its deinit method. +// This is fine as long as its deinit isn't called during cache reset +final class Product: Sendable { + deinit { + @Dependency(NestedDependency.self) var nested + _ = nested + } +} + +// Factory dependency that vends Product instances +protocol Factory: Sendable { + func makeProduct() -> Product +} + +enum FactoryDependency: DependencyKey { + static let liveValue: Factory = LiveFactory() + static var testValue: Factory { MockFactory() } +} + +// Live factory instantiates a new product for each call +struct LiveFactory: Factory { + func makeProduct() -> Product { Product() } +} + +// Mock factory holds onto a mock Product. +// This results in the mock Product being released during cache reset. +final class MockFactory: Factory { + let mockProduct = LockIsolated(Product()) + + func makeProduct() -> Product { mockProduct.value } +}