From cb9c1f845b3b981da787452beee893826210eb7a Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Tue, 12 Dec 2023 14:07:02 -0800 Subject: [PATCH] Revert "Fix Store leak when async effect is in flight (#2643)" (#2648) * wip * wip --- Sources/ComposableArchitecture/Store.swift | 83 +++++++++---------- .../StoreLifetimeTests.swift | 6 ++ 2 files changed, 44 insertions(+), 45 deletions(-) diff --git a/Sources/ComposableArchitecture/Store.swift b/Sources/ComposableArchitecture/Store.swift index 6e3fdde3ab54..0ad7ad3678e3 100644 --- a/Sources/ComposableArchitecture/Store.swift +++ b/Sources/ComposableArchitecture/Store.swift @@ -485,10 +485,6 @@ public final class Store { for id in self.children.keys { self.invalidateChild(id: id) } - self.effectCancellables.values.forEach { cancellable in - cancellable.cancel() - } - self.effectCancellables.removeAll() } fileprivate func invalidateChild(id: ScopeID) { @@ -530,7 +526,6 @@ public final class Store { defer { index += 1 } let action = self.bufferedActions[index] let effect = self.reducer.reduce(into: ¤tState, action: action) - let uuid = UUID() switch effect.operation { case .none: @@ -538,6 +533,7 @@ public final class Store { case let .publisher(publisher): var didComplete = false let boxedTask = Box?>(wrappedValue: nil) + let uuid = UUID() let effectCancellable = withEscapedDependencies { continuation in publisher .handleEvents( @@ -575,48 +571,45 @@ public final class Store { } case let .run(priority, operation): withEscapedDependencies { continuation in - let task = Task(priority: priority) { @MainActor [weak self] in - #if DEBUG - let isCompleted = LockIsolated(false) - defer { isCompleted.setValue(true) } - #endif - await operation( - Send { effectAction in - #if DEBUG - if isCompleted.value { - runtimeWarn( - """ - An action was sent from a completed effect: - - Action: - \(debugCaseOutput(effectAction)) - - Effect returned from: - \(debugCaseOutput(action)) - - Avoid sending actions using the 'send' argument from 'Effect.run' after \ - the effect has completed. This can happen if you escape the 'send' \ - argument in an unstructured context. - - To fix this, make sure that your 'run' closure does not return until \ - you're done calling 'send'. - """ - ) + tasks.wrappedValue.append( + Task(priority: priority) { @MainActor in + #if DEBUG + let isCompleted = LockIsolated(false) + defer { isCompleted.setValue(true) } + #endif + await operation( + Send { effectAction in + #if DEBUG + if isCompleted.value { + runtimeWarn( + """ + An action was sent from a completed effect: + + Action: + \(debugCaseOutput(effectAction)) + + Effect returned from: + \(debugCaseOutput(action)) + + Avoid sending actions using the 'send' argument from 'Effect.run' after \ + the effect has completed. This can happen if you escape the 'send' \ + argument in an unstructured context. + + To fix this, make sure that your 'run' closure does not return until \ + you're done calling 'send'. + """ + ) + } + #endif + if let task = continuation.yield({ + self.send(effectAction, originatingFrom: action) + }) { + tasks.wrappedValue.append(task) } - #endif - if let task = continuation.yield({ - self?.send(effectAction, originatingFrom: action) - }) { - tasks.wrappedValue.append(task) } - } - ) - self?.effectCancellables[uuid] = nil - } - tasks.wrappedValue.append(task) - self.effectCancellables[uuid] = AnyCancellable { - task.cancel() - } + ) + } + ) } } } diff --git a/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift b/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift index 15cc45ac77f3..ff5291ce0fb2 100644 --- a/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift +++ b/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift @@ -64,6 +64,9 @@ final class StoreLifetimeTests: BaseTCATestCase { } func testStoreDeinit_RunningEffect() async { + XCTTODO( + "We would like for this to pass, but it requires full deprecation of uncached child stores" + ) Logger.shared.isEnabled = true let effectFinished = self.expectation(description: "Effect finished") do { @@ -90,6 +93,9 @@ final class StoreLifetimeTests: BaseTCATestCase { } func testStoreDeinit_RunningCombineEffect() async { + XCTTODO( + "We would like for this to pass, but it requires full deprecation of uncached child stores" + ) Logger.shared.isEnabled = true let effectFinished = self.expectation(description: "Effect finished") do {