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.

Sign up for GitHub

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

Prevent potential simultaneous access to cached values during cache reset #351

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Prevent potential simultaneous access to cached values during cache reset #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49d955b
Prevent potential simultaneous access to cached values during cache r…
jflan-dd Mar 12, 2025
3acf746
Update cache reset test to more closely match actual usage
jflan-dd Mar 13, 2025
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
222 changes: 117 additions & 105 deletions Sources/Dependencies/DependencyValues.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<Key: TestDependencyKey>(
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}

Expand All @@ -618,6 +580,56 @@ public final class CachedValues: @unchecked Sendable {
}
}

public func reportDependencyIssue<Key: TestDependencyKey>(
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
Expand Down
71 changes: 71 additions & 0 deletions Tests/DependenciesTests/SimultaneousCacheAccessTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 }
}