Skip to content

Commit

Permalink
Fix potential deadlock in Shared (#3356)
Browse files Browse the repository at this point in the history
* Fixed shared deadlock.

* Fix deadlock>

* wip

* wip

* Improve test.
  • Loading branch information
mbrandonw committed Sep 6, 2024
1 parent 105bc0f commit 2c8b1c0
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 1 deletion.
14 changes: 14 additions & 0 deletions Sources/ComposableArchitecture/Internal/DispatchQueue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ func mainActorNow<R: Sendable>(execute block: @MainActor @Sendable () -> R) -> R
}
}

func mainActorASAP(execute block: @escaping @MainActor @Sendable () -> Void) {
if DispatchQueue.getSpecific(key: key) == value {
MainActor._assumeIsolated {
block()
}
} else {
DispatchQueue.main.async {
MainActor._assumeIsolated {
block()
}
}
}
}

private let key: DispatchSpecificKey<UInt8> = {
let key = DispatchSpecificKey<UInt8>()
DispatchQueue.main.setSpecific(key: key, value: value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
}
}

#if DEBUG
private final class BindableActionDebugger<Action>: Sendable {
let isInvalidated: @MainActor @Sendable () -> Bool
let value: any Sendable
Expand Down Expand Up @@ -112,6 +113,7 @@
}
}
}
#endif

extension BindableAction where State: ObservableState {
fileprivate static func set<Value: Equatable & Sendable>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ final class ValueReference<Value, Persistence: PersistenceReaderKey<Value>>: Ref
initialValue: initialValue
) { [weak self] value in
guard let self else { return }
mainActorNow {
mainActorASAP {
#if canImport(Perception)
self._$perceptionRegistrar.willSet(self, keyPath: \.value)
defer { self._$perceptionRegistrar.didSet(self, keyPath: \.value) }
Expand Down
41 changes: 41 additions & 0 deletions Tests/ComposableArchitectureTests/SharedTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,47 @@ final class SharedTests: XCTestCase {
}
}
}

func testReEntrantSharedSubscriptionDependencyResolution() async throws {
for _ in 1...100 {
try await withDependencies {
$0 = DependencyValues()
} operation: {
@Shared(.appStorage("count")) var count = 0

struct Client: TestDependencyKey {
init() {
@Dependency(\.defaultAppStorage) var userDefaults
userDefaults.set(42, forKey: "count")
}
static var testValue: Self { Self() }
}

withEscapedDependencies { dependencies in
DispatchQueue.global().async {
dependencies.yield {
XCTAssertEqual({ Thread.isMainThread }(), false)
@Dependency(Client.self) var client
_ = client
}
}
DispatchQueue.main.async { [sharedCount = $count] in
dependencies.yield {
XCTAssertEqual({ Thread.isMainThread }(), true)
_ = sharedCount.wrappedValue
}
}
}

try await Task.sleep(nanoseconds: 10_000_000)
XCTAssertEqual(count, 42)
}
}
}
}

@globalActor actor GA: GlobalActor {
static let shared = GA()
}

@Reducer
Expand Down

0 comments on commit 2c8b1c0

Please sign in to comment.