Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

TCA holds on to state for longer than expected #3045

Closed
2 of 3 tasks
pfandrade opened this issue May 5, 2024 · 1 comment
Closed
2 of 3 tasks

TCA holds on to state for longer than expected #3045

pfandrade opened this issue May 5, 2024 · 1 comment

Comments

@pfandrade
Copy link

Description

I'm finishing my first app with TCA and was trying to track down a memory leak. While doing so I found an unexpected behaviour in TCA:

If a sheet is presented and then dismissed, the associated feature state is not released after the dismiss. Only after the sheet is presented again is the state released. It seems the previous state is being held somewhere within TCA.

I've used the SyncUps example to confirm it's not something in my project. Here's a patch for the simple test I made:

diff --git a/Examples/SyncUps/SyncUps/SyncUpForm.swift b/Examples/SyncUps/SyncUps/SyncUpForm.swift
index 4e5e25e8d..b099926c6 100644
--- a/Examples/SyncUps/SyncUps/SyncUpForm.swift
+++ b/Examples/SyncUps/SyncUps/SyncUpForm.swift
@@ -8,7 +8,8 @@ struct SyncUpForm {
   struct State: Equatable, Sendable {
     var focus: Field? = .title
     var syncUp: SyncUp
-
+    var disposable = Disposable { print("DEINIT") }
+    
     init(focus: Field? = .title, syncUp: SyncUp) {
       self.focus = focus
       self.syncUp = syncUp
@@ -22,6 +23,8 @@ struct SyncUpForm {
       case attendee(Attendee.ID)
       case title
     }
+      
+      
   }
 
   enum Action: BindableAction, Equatable, Sendable {
@@ -136,3 +139,18 @@ extension Duration {
     )
   }
 }
+
+class Disposable: Equatable {
+    static func == (lhs: Disposable, rhs: Disposable) -> Bool {
+        lhs === rhs
+    }
+    
+    init(_ onDispose: @escaping () -> Void) {
+        self.onDispose = onDispose
+    }
+    var onDispose: () -> Void
+    
+    deinit {
+        onDispose()
+    }
+}

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

I would expect that "DEINIT" is printed when the SyncUpForm is dismissed.

Actual behavior

"DEINIT" is only printed after the second time the sheet is presented.

Steps to reproduce

  1. Apply the patch above
  2. Add a new sync up
  3. Dismiss it
  4. Confirm DEINIT is not printed

The Composable Architecture version information

5d73967

Destination operating system

iOS 17

Xcode version information

Version 15.3 (15E204a)

Swift Compiler version information

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0
@pfandrade pfandrade added the bug Something isn't working due to a bug in the library. label May 5, 2024
@stephencelis
Copy link
Member

@pfandrade This is a behavior of SwiftUI navigation: when a screen is presented and dismissed, often times the destination view and state are kept around in memory till the next presentation. You can take our vanilla SyncUps app that uses plain old observable models and add this to SyncUpFormModel and you'll see the same behavior:

deinit {
  print("DEINIT")
}

And the behavior is reproducible in much simpler apps, as well.

So I do not think you should depend on the lifetime of objects held in the view graph being tied to the lifetime of views on the screen, and instead you can depend on other, more predictable hooks.

Since this isn't a bug in TCA, and is just vanilla SwiftUI behavior, I'm going to convert this to a discussion.

@stephencelis stephencelis removed the bug Something isn't working due to a bug in the library. label May 5, 2024
@pointfreeco pointfreeco locked and limited conversation to collaborators May 5, 2024
@stephencelis stephencelis converted this issue into discussion #3046 May 5, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants