Skip to content

Commit 4af9827

Browse files
committed
Avoid deadlock when accessing @Shared in a @Dependency
...by not holding locks while accessing `Dependency` logic with locks of its own. Also simplify `PersistentReferences` implementation by maintaining weak references to `_PersistentReference<Key>` objects directly, without custom reference counting. Fixes #149.
1 parent f675de1 commit 4af9827

File tree

3 files changed

+30
-116
lines changed

3 files changed

+30
-116
lines changed

Sources/Sharing/Internal/PersistentReferences.swift

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ final class PersistentReferences: @unchecked Sendable, DependencyKey {
66
static var liveValue: PersistentReferences { PersistentReferences() }
77
static var testValue: PersistentReferences { PersistentReferences() }
88

9-
struct Pair<Key: SharedReaderKey> {
10-
var cachedValue: Key.Value
11-
var reference: _PersistentReference<Key>?
9+
struct Weak<Key: SharedReaderKey> {
10+
weak var reference: _PersistentReference<Key>?
1211
}
1312

1413
private var storage: [AnyHashable: Any] = [:]
@@ -18,37 +17,35 @@ final class PersistentReferences: @unchecked Sendable, DependencyKey {
1817
forKey key: Key,
1918
default value: @autoclosure () throws -> Key.Value,
2019
skipInitialLoad: Bool
21-
) rethrows -> _ManagedReference<Key> {
22-
try lock.withLock {
23-
guard var pair = storage[key.id] as? Pair<Key> else {
24-
let value = try value()
25-
let persistentReference = _PersistentReference(
20+
) rethrows -> _PersistentReference<Key> {
21+
guard let reference = lock.withLock({ (storage[key.id] as? Weak<Key>)?.reference }) else {
22+
let value = try value()
23+
return withExtendedLifetime(
24+
_PersistentReference(
2625
key: key,
2726
value: value,
2827
skipInitialLoad: skipInitialLoad
2928
)
30-
storage[key.id] = Pair(cachedValue: value, reference: persistentReference)
31-
return _ManagedReference(persistentReference)
29+
) { reference in
30+
lock.withLock {
31+
if let reference = (storage[key.id] as? Weak<Key>)?.reference {
32+
return reference
33+
} else {
34+
storage[key.id] = Weak(reference: reference)
35+
reference.onDeinit = { [self] in
36+
removeReference(forKey: key)
37+
}
38+
return reference
39+
}
40+
}
3241
}
33-
guard let persistentReference = pair.reference else {
34-
let persistentReference = _PersistentReference(
35-
key: key,
36-
value: skipInitialLoad ? (try? value()) ?? pair.cachedValue : pair.cachedValue,
37-
skipInitialLoad: skipInitialLoad
38-
)
39-
pair.reference = persistentReference
40-
storage[key.id] = pair
41-
return _ManagedReference(persistentReference)
42-
}
43-
return _ManagedReference(persistentReference)
4442
}
43+
return reference
4544
}
4645

4746
func removeReference<Key: SharedReaderKey>(forKey key: Key) {
4847
lock.withLock {
49-
guard var pair = storage[key.id] as? Pair<Key> else { return }
50-
pair.reference = nil
51-
storage[key.id] = pair
48+
_ = storage.removeValue(forKey: key.id)
5249
}
5350
}
5451
}

Sources/Sharing/Internal/Reference.swift

Lines changed: 5 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ final class _PersistentReference<Key: SharedReaderKey>:
196196
private var _saveError: (any Error)?
197197
private var _referenceCount = 0
198198
private var subscription: SharedSubscription?
199+
internal var onDeinit: (() -> Void)?
199200

200201
init(key: Key, value initialValue: Key.Value, skipInitialLoad: Bool) {
201202
self.key = key
@@ -231,6 +232,10 @@ final class _PersistentReference<Key: SharedReaderKey>:
231232
)
232233
}
233234

235+
deinit {
236+
onDeinit?()
237+
}
238+
234239
var id: ObjectIdentifier { ObjectIdentifier(self) }
235240

236241
var isLoading: Bool {
@@ -313,16 +318,6 @@ final class _PersistentReference<Key: SharedReaderKey>:
313318
lock.withLock { _referenceCount += 1 }
314319
}
315320

316-
func release() {
317-
let shouldRelease = lock.withLock {
318-
_referenceCount -= 1
319-
return _referenceCount <= 0
320-
}
321-
guard shouldRelease else { return }
322-
@Dependency(PersistentReferences.self) var persistentReferences
323-
persistentReferences.removeReference(forKey: key)
324-
}
325-
326321
func access<Member>(
327322
keyPath: KeyPath<_PersistentReference, Member>,
328323
fileID: StaticString = #fileID,
@@ -454,85 +449,6 @@ extension _PersistentReference: MutableReference, Equatable where Key: SharedKey
454449
}
455450
}
456451

457-
final class _ManagedReference<Key: SharedReaderKey>: Reference, Observable {
458-
private let base: _PersistentReference<Key>
459-
460-
init(_ base: _PersistentReference<Key>) {
461-
base.retain()
462-
self.base = base
463-
}
464-
465-
deinit {
466-
base.release()
467-
}
468-
469-
var id: ObjectIdentifier {
470-
base.id
471-
}
472-
473-
var isLoading: Bool {
474-
base.isLoading
475-
}
476-
477-
var loadError: (any Error)? {
478-
base.loadError
479-
}
480-
481-
var wrappedValue: Key.Value {
482-
base.wrappedValue
483-
}
484-
485-
func load() async throws {
486-
try await base.load()
487-
}
488-
489-
func touch() {
490-
base.touch()
491-
}
492-
493-
#if canImport(Combine)
494-
var publisher: any Publisher<Key.Value, Never> {
495-
base.publisher
496-
}
497-
#endif
498-
499-
var description: String {
500-
base.description
501-
}
502-
}
503-
504-
extension _ManagedReference: MutableReference, Equatable where Key: SharedKey {
505-
var saveError: (any Error)? {
506-
base.saveError
507-
}
508-
509-
var snapshot: Key.Value? {
510-
base.snapshot
511-
}
512-
513-
func takeSnapshot(
514-
_ value: Key.Value,
515-
fileID: StaticString,
516-
filePath: StaticString,
517-
line: UInt,
518-
column: UInt
519-
) {
520-
base.takeSnapshot(value, fileID: fileID, filePath: filePath, line: line, column: column)
521-
}
522-
523-
func withLock<R>(_ body: (inout Key.Value) throws -> R) rethrows -> R {
524-
try base.withLock(body)
525-
}
526-
527-
func save() async throws {
528-
try await base.save()
529-
}
530-
531-
static func == (lhs: _ManagedReference, rhs: _ManagedReference) -> Bool {
532-
lhs.base == rhs.base
533-
}
534-
}
535-
536452
final class _AppendKeyPathReference<
537453
Base: Reference, Value, Path: KeyPath<Base.Value, Value> & Sendable
538454
>: Reference, Observable {

Sources/Sharing/SharedContinuations.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ private final class ContinuationBox<Value>: Sendable {
229229
invoked. This will cause tasks waiting on it to resume immediately.
230230
"""
231231
)
232-
callback.withLock { $0?(.success(nil)) }
232+
callback.withLock(\.self)?(.success(nil))
233233
}
234234
}
235235

@@ -246,9 +246,10 @@ private final class ContinuationBox<Value>: Sendable {
246246
)
247247
return
248248
}
249-
callback.withLock { callback in
249+
let callback = callback.withLock { callback in
250250
defer { callback = nil }
251-
callback?(result)
251+
return callback
252252
}
253+
callback?(result)
253254
}
254255
}

0 commit comments

Comments
 (0)