Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix caching of dependencies during re-entrancy #249

Merged
merged 9 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 74 additions & 70 deletions Sources/Dependencies/DependencyValues.swift
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ public struct DependencyValues: Sendable {
@TaskLocal static var isSetting = false
@TaskLocal static var currentDependency = CurrentDependency()

fileprivate var cachedValues = CachedValues()
@_spi(Internals)
public var cachedValues = CachedValues()
private var storage: [ObjectIdentifier: any Sendable] = [:]

/// Creates a dependency values instance.
Expand All @@ -140,7 +141,7 @@ public struct DependencyValues: Sendable {
.takeUnretainedValue()
else { return }
let testCaseWillStartBlock: @convention(block) (AnyObject) -> Void = { _ in
DependencyValues._current.cachedValues.cached.withValue { $0 = [:] }
DependencyValues._current.cachedValues.cached = [:]
}
let testCaseWillStartImp = imp_implementationWithBlock(testCaseWillStartBlock)
class_addMethod(
Expand All @@ -154,7 +155,7 @@ public struct DependencyValues: Sendable {
if _XCTIsTesting {
XCTestObservationCenter.shared.addTestObserver(
TestObserver({
DependencyValues._current.cachedValues.cached.withValue { $0 = [:] }
DependencyValues._current.cachedValues.cached = [:]
}))
}
#else
Expand All @@ -177,7 +178,7 @@ public struct DependencyValues: Sendable {
}
#endif
pRegisterTestObserver?({
DependencyValues._current.cachedValues.cached.withValue { $0 = [:] }
DependencyValues._current.cachedValues.cached = [:]
})
#endif
}
Expand Down Expand Up @@ -336,13 +337,15 @@ private let defaultContext: DependencyContext = {
}
}()

private final class CachedValues: Sendable {
struct CacheKey: Hashable, Sendable {
@_spi(Internals)
public final class CachedValues: @unchecked Sendable {
public struct CacheKey: Hashable, Sendable {
let id: ObjectIdentifier
let context: DependencyContext
}

fileprivate let cached = LockIsolated([CacheKey: any Sendable]())
private let lock = NSRecursiveLock()
public var cached = [CacheKey: any Sendable]()

func value<Key: TestDependencyKey>(
for key: Key.Type,
Expand All @@ -351,90 +354,91 @@ private final class CachedValues: Sendable {
function: StaticString = #function,
line: UInt = #line
) -> Key.Value {
XCTFailContext.$current.withValue(XCTFailContext(file: file, line: line)) {
self.cached.withValue { cached in
let cacheKey = CacheKey(id: ObjectIdentifier(key), context: context)
guard let base = cached[cacheKey], let value = base as? Key.Value
else {
let value: Key.Value?
switch context {
case .live:
value = (key as? any DependencyKey.Type)?.liveValue as? Key.Value
case .preview:
value = Key.previewValue
case .test:
value = Key.testValue
}
lock.lock()
defer { lock.unlock() }

return XCTFailContext.$current.withValue(XCTFailContext(file: file, line: line)) {
let cacheKey = CacheKey(id: ObjectIdentifier(key), context: context)
guard let base = cached[cacheKey], let value = base as? Key.Value
else {
let value: Key.Value?
switch context {
case .live:
value = (key as? any DependencyKey.Type)?.liveValue as? Key.Value
case .preview:
value = Key.previewValue
case .test:
value = Key.testValue
}

guard let value
else {
#if DEBUG
if !DependencyValues.isSetting {
var dependencyDescription = ""
if let fileID = DependencyValues.currentDependency.fileID,
let line = DependencyValues.currentDependency.line
{
dependencyDescription.append(
guard let value
else {
#if DEBUG
if !DependencyValues.isSetting {
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
? """
)
}
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(_:)" ? "\(typeName(Key.self)).self" : "\\.\(function)"
}

runtimeWarn(
"""
@Dependency(\(argument)) has no live implementation, but was accessed from a live \
context.

\(dependencyDescription)

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.

* 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.
""",
file: DependencyValues.currentDependency.file ?? file,
line: DependencyValues.currentDependency.line ?? line
)
}
#endif
let value = Key.testValue
if !DependencyValues.isSetting {
cached[cacheKey] = value
)

var argument: String {
"\(function)" == "subscript(_:)" ? "\(typeName(Key.self)).self" : "\\.\(function)"
}
return value
}

cached[cacheKey] = value
runtimeWarn(
"""
@Dependency(\(argument)) has no live implementation, but was accessed from a live \
context.

\(dependencyDescription)

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.

* 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.
""",
file: DependencyValues.currentDependency.file ?? file,
line: DependencyValues.currentDependency.line ?? line
)
}
#endif
let value = Key.testValue
if !DependencyValues.isSetting {
cached[cacheKey] = value
}
return value
}

cached[cacheKey] = value
return value
}

return value
}
}
}
44 changes: 44 additions & 0 deletions Tests/DependenciesTests/CacheTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
@_spi(Internals) import Dependencies
import XCTest

final class CachedValueTests: XCTestCase {
override func tearDown() {
super.tearDown()
CacheLocals.$skipFailure.withValue(true) {
DependencyValues._current.cachedValues.cached = [:]
}
}

func testCacheWithReEntrantAccess() {
@Dependency(OuterDependencyTests.self) var outerDependency
_ = outerDependency
}
}

private struct OuterDependencyTests: TestDependencyKey {
static var testValue: OuterDependencyTests {
@Dependency(InnerDependency.self) var innerDependency
_ = innerDependency
return Self()
}
}
private struct InnerDependency: TestDependencyKey {
let perform: @Sendable () -> Void
static var testValue: InnerDependency {
final class Ref: Sendable {
deinit {
guard !CacheLocals.skipFailure
else { return }
XCTFail("This should not deinit")
}
}
let ref = Ref()
return Self {
_ = ref
}
}
}

private enum CacheLocals {
@TaskLocal static var skipFailure = false
}