Skip to content

Commit

Permalink
Revert "Fix Store leak when async effect is in flight (pointfreeco#2643
Browse files Browse the repository at this point in the history
…)" (pointfreeco#2648)

* wip

* wip
  • Loading branch information
stephencelis authored Dec 12, 2023
1 parent b86012c commit cb9c1f8
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 45 deletions.
83 changes: 38 additions & 45 deletions Sources/ComposableArchitecture/Store.swift
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,6 @@ public final class Store<State, Action> {
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<State, Action>) {
Expand Down Expand Up @@ -530,14 +526,14 @@ public final class Store<State, Action> {
defer { index += 1 }
let action = self.bufferedActions[index]
let effect = self.reducer.reduce(into: &currentState, action: action)
let uuid = UUID()

switch effect.operation {
case .none:
break
case let .publisher(publisher):
var didComplete = false
let boxedTask = Box<Task<Void, Never>?>(wrappedValue: nil)
let uuid = UUID()
let effectCancellable = withEscapedDependencies { continuation in
publisher
.handleEvents(
Expand Down Expand Up @@ -575,48 +571,45 @@ public final class Store<State, Action> {
}
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()
}
)
}
)
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions Tests/ComposableArchitectureTests/StoreLifetimeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit cb9c1f8

Please sign in to comment.