pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
11.99k stars 1.42k forks source link

StackState/StackReducer related leak #2508

Open bioche opened 10 months ago

bioche commented 10 months ago

Description

For the last 2 days I'v been trying to understand a leak & I have been able to reproduce it in this sample project : NavigationStackLeak.zip

The issue occurs when creating a StoreOf<FlowCore> from scratch, where FlowCore is a Reducer wrapping a StackState / StackAction combined using forEach operator.

It seems like pushing a node in StackState causes effects to be added by _StackReducer that will cause Store & all its properties to not being deallocated once the NavigationStack gets dismissed causing a leak

Thanks in advance for looking into this :)

Checklist

Expected behavior

I would expect Store to be removed from memory once the view/controller using them has been dismissed

Actual behavior

Stores are stacking up after each present/dismiss of FlowView

Steps to reproduce

  1. Launch attached project app : NavigationStackLeak.zip
  2. Tap "present stack" button
  3. Tap "go to next screen"
  4. Dismiss modal using cross or swipe down gesture
  5. Repeat 2 or 3 times previous steps
  6. Open memory inspector & filter with FlowCore : you will see as many instance of StoreOf<FlowCore> as modal opened
Screenshot 2023-10-07 at 22 16 25

It's important to note that the issue is not there when FlowCore is embedded in another Reducer like MainCore in example project (try same scenario using "present stack using TCA" button instead). probably because effects built in _StackReducer get properly cancelled in that case.

A quick fix on my side would be to send action to pop all nodes of StackState when receiving dismiss action. However a fix on the lib would be preferable as others could stumble upon this issue & like me waste hours trying to figure out what is happening ;)

The Composable Architecture version information

0.59.0 / 1.2.0 / main branch

Destination operating system

iOS 16.2

Xcode version information

XCode 14.2

Swift Compiler version information

swift-driver version: 1.62.15 Apple Swift version 5.7.2 (swiftlang-5.7.2.135.5 clang-1400.0.29.51)
Target: arm64-apple-macosx13.0
mbrandonw commented 10 months ago

Hi @bioche, are you certain it is an issue in TCA and not that SwiftUI is holding onto the Store even after the view has been dismissed? We have seen that many times in the past.

I ran your sample project in Xcode 15 with the iOS 17 simulator and I do not see any leaks in the memory graph debugger. Perhaps it was fixed in iOS 17?

bioche commented 10 months ago

Thanks @mbrandonw for the quick response !

I'm not sure it's SwiftUI as when using the "present stack using TCA" that uses the same views but with a MainCore that has a ifLet reducer it works with no leak.

Also I forgot to mention in the bug report that if I comment those 2 effects in the _StackReducer the leak doesn't occur either :

Screenshot 2023-10-07 at 23 07 47

I reproduce it on iOS 17 simulator on my side : https://github.com/pointfreeco/swift-composable-architecture/assets/4895727/2088271f-e7f2-4dac-821a-d69b6754d681

stephencelis commented 7 months ago

Reopening with #2648. We will be able to fix this eventually, but for now there is a small cycle that in breaking can prevent certain effects from making it back into the system.