Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential memory leak in child store caching #3634

Closed
3 tasks done
Kolos65 opened this issue Mar 25, 2025 · 6 comments · Fixed by #3646
Closed
3 tasks done

Potential memory leak in child store caching #3634

Kolos65 opened this issue Mar 25, 2025 · 6 comments · Fixed by #3646
Labels
bug Something isn't working due to a bug in the library.

Comments

@Kolos65
Copy link

Kolos65 commented Mar 25, 2025

Description

Upon investigating a performance issue in my app, I found a potential leak related to child store caching. 

I am developing a transcription app with a very basic navigation setup:

  • there is a root screen with a list of projects
  • you can tap a project that pushes a project edit screen onto the stack
  • the project edit screen is quite complex with many child features (it operates on an array of subtitles, each having its own reducer)
  • I use NavigationStack(path:destination:) and a Path enum reducer with StackState and StackAction in the main feature

When navigating back to the root screen after opening a project, I would expect all child states to be removed from memory. Instead I can see:

  • multiple instances of ObservableStateID.Storage
  • multiple instances of ObservationRegistrar.Extent

Both Storage and Extent are held by what seems to be the child feature’s state (which I would expect having been deallocated at this point). The number of instances in both cases correlates with the number of child features I create when opening a project. So if I open and close a project X times, you would see X times more instances of these in the memory debugger.



I also noticed that setting Store.canCacheChildren to have a default false starting value will resolve the above leak. This indicates that the issue is related to caching child states in the scope implementation of the store:

public final class Store<State, Action> {
  // Setting to false resolves the leak
  var canCacheChildren = true
  // Keeps child states in memory after lifecycle ends?
  private var children: [ScopeID<State, Action>: AnyObject] = [:]
...

According to LLMs, storing strong references to child stores in the children array is a memory leak. I know things are often more nuanced (thus the “potential” in the title), but when and who removes child stores from that children array?

I am happy to provide more context if needed.

Memory after popping back to the root with Store.canCacheChildren = true:
Image
Memory after popping back to the root with Store.canCacheChildren = false:
Image
Storage memory graph:
Image
Extent memory graph:
Image

Checklist

  • I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
  • If possible, I've reproduced the issue using the main branch of this package.
  • This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

No response

Actual behavior

No response

Reproducing project

No response

The Composable Architecture version information

1.18.0

Destination operating system

iOS 18.0

Xcode version information

Xcode 16.2

Swift Compiler version information

swift-driver version: 1.115.1 Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)
Target: arm64-apple-macosx15.0
@Kolos65 Kolos65 added the bug Something isn't working due to a bug in the library. label Mar 25, 2025
@stephencelis
Copy link
Member

@Kolos65 This is a known issue that we will be able to address when some of TCA's reducer machinery is reimplemented for 2.0. In practice I believe these extra stores should be mostly harmless, though. Are you experiencing an actual problem from them sticking around?

@Kolos65
Copy link
Author

Kolos65 commented Mar 25, 2025

@stephencelis Yes, performance is degraded heavily due to this. After opening a project ~10 times, there are severe frame drops.

Not sure though if I am not misusing something as I have a large number of child features and a couple of actions sent by gestures, but I was able to correlate this to the child store caching by not being able to repro the performance degradation with Store.canCacheChildren = false

@stephencelis
Copy link
Member

@Kolos65 I wonder if you could cook up a repro for us to investigate? Something that emulates the conditions of your app in a small parent/child relationship? Ideally the invalidated stores are simply orphans that don't affect the performance of an application at all.

@Kolos65
Copy link
Author

Kolos65 commented Apr 4, 2025

@stephencelis I managed to create a minimal repro for you. I have set up an example with a parent and a detail feature. The detail adds a large number of child features and child views. The child views are little circles which you can drag around with a gesture. The gesture fires a child action to update the child view position in the child state. You can notice the performance issue after opening the detail 10 times (only with canCacheChildren = true):

import SwiftUI
import ComposableArchitecture

// MARK: - App

@main
struct ExampleApp: App {
    var body: some Scene {
        WindowGroup {
            AppView(store: Store(initialState: AppFeature.State()) {
                AppFeature()
            })
        }
    }
}

// MARK: - App Feature

@Reducer
struct AppFeature {
    @ObservableState
    struct State: Equatable {
        var path = StackState<Path.State>()
    }

    enum Action {
        case path(StackActionOf<Path>)
        case openDetailButtonTapped
        case openDetail10xButtonTapped
        case pushDetail
        case popDetail
    }

    @Reducer(state: .equatable)
    enum Path {
        case detail(DetailFeature)
    }

    @Dependency(\.continuousClock) var clock

    var body: some ReducerOf<Self> {
        Reduce { state, action in
            switch action {
            case .openDetailButtonTapped:
                return .send(.pushDetail)
            case .openDetail10xButtonTapped:
                return .run { send in
                    for _ in (0..<9) {
                        await send(.pushDetail)
                        try await clock.sleep(for: .milliseconds(500))
                        await send(.popDetail)
                        try await clock.sleep(for: .milliseconds(500))
                    }
                    await send(.pushDetail)
                }
            case .pushDetail:
                state.path.append(.detail(DetailFeature.State()))
                return .none
            case .popDetail:
                _ = state.path.popLast()
                return .none
            case .path:
                return .none
            }
        }
        .forEach(\.path, action: \.path)
    }
}

struct AppView: View {
    @Bindable var store: StoreOf<AppFeature>
    var body: some View {
        NavigationStack(
            path: $store.scope(state: \.path, action: \.path)
        ) {
            VStack {
                Button("Open Detail") {
                    store.send(.openDetailButtonTapped)
                }
                .buttonStyle(.borderedProminent)
                Button("Open Detail 10x") {
                    store.send(.openDetail10xButtonTapped)
                }
                .buttonStyle(.borderedProminent)
            }
        } destination: { store in
            switch store.case {
            case .detail(let store):
                DetailView(store: store)
            }
        }
    }
}

// MARK: - Detail Feature

@Reducer
struct DetailFeature {
    @ObservableState
    struct State: Equatable {
        var childs: IdentifiedArrayOf<ChildFeature.State>
        init() {
            let childs = (0..<500).map { _ in ChildFeature.State() }
            self.childs = IdentifiedArray(uniqueElements: childs)
        }
    }

    enum Action {
        case childs(IdentifiedActionOf<ChildFeature>)
    }

    var body: some ReducerOf<Self> {
        EmptyReducer()
            .forEach(\.childs, action: \.childs) {
                ChildFeature()
            }
    }
}

struct DetailView: View {
    let store: StoreOf<DetailFeature>
    var body: some View {
        ZStack {
            ForEach(store.childs) { child in
                if let store = self.store.scope(
                    state: \.childs[id: child.id],
                    action: \.childs[id: child.id]
                ) {
                    ChildView(store: store)
                }
            }
        }
        .frame(maxWidth: .infinity, maxHeight: .infinity)
    }
}

// MARK: - Child Feature

@Reducer
struct ChildFeature {
    @ObservableState
    struct State: Equatable, Identifiable {
        var id = UUID()
        var position: CGPoint = .init(
            x: CGFloat((0...1000).randomElement()!),
            y: CGFloat((0...1000).randomElement()!)
        )
        var isMoving = false
    }

    enum Action {
        case moved(CGPoint)
        case released
    }

    var body: some ReducerOf<Self> {
        Reduce { state, action in
            switch action {
            case .moved(let point):
                state.isMoving = true
                state.position = point
                return .none
            case .released:
                state.isMoving = false
                return .none
            }
        }
    }
}

struct ChildView: View {
    let store: StoreOf<ChildFeature>

    var body: some View {
        Circle()
            .fill(store.isMoving ? .blue : .gray)
            .stroke(.black, lineWidth: 2)
            .frame(width: 50)
            .position(x: store.position.x, y: store.position.y)
            .zIndex(store.isMoving ? 1 : 0)
            .gesture(dragGesture)
    }

    var dragGesture: some Gesture {
        DragGesture(minimumDistance: 0)
            .onChanged { value in
                store.send(.moved(value.location))
            }
            .onEnded { _ in
                store.send(.released)
            }
    }
}

@stephencelis
Copy link
Member

@Kolos65 Thanks for the repro! It was really helpful. We think we have a potential fix pushed to core-firehose-isinvalid. Can you point your project there and see if it helps?

@Kolos65
Copy link
Author

Kolos65 commented Apr 4, 2025

@stephencelis Thank you for the quick turnaround. 😊

I’ve tested my project using the core-firehose-isinvalid branch, and I’m happy to report that the performance issue appears to be resolved.

The fix looks very promising, during my earlier investigation with Instruments, I had come across subscribeToDidSet as a potential hotspot, though at the time I was focused on tracking down a memory leak and didn’t push it further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working due to a bug in the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants