Skip to content

Observed view model leaks when using UIViewController.present(item: ...) #283

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

Open
3 tasks done
lucolivier-dumaisblais opened this issue Apr 1, 2025 · 2 comments
Open
3 tasks done
Labels
bug Something isn't working

Comments

@lucolivier-dumaisblais
Copy link

lucolivier-dumaisblais commented Apr 1, 2025

Description

Hey folks, I'm working on refactoring a big portion of our mixed SwiftUI and UIKit. To address the UIKit part, I'm slowing adopting UIKitNavigation but I'm running into a memory leak that I struggle to see how to fix.

From my understanding of the problem the present helper is creating a retain cycle on the observed object.

I've created a sample project demonstrating the leak. It includes a possible fix though it is less than ideal for general adoption of UIKitNavigation in our codebase.

Here's a few explanations for the sample project

To simplify the problem, I've created a simple example UIKit containing two VC
Root and ChildFeature. Root is based on vanilla UIKit code and patterns.
ChildFeature is the hierarchy of VCs I'm slowing refactoring to adopt state-based navigation and my first feature using it is leaking its view model once ChildFeatureVC is dismissed by the Root.

The issue I end up having is that If I present ChildFeatureViewController , user does its thing in that feature, then dismisses it. The ChildFeatureViewModel is retaining itself forever. So everytime I present, I end up with a new instance of the VM leaking.
I struggle to see how to resolve this retain cycle. The only thing I figured out is "emitting" a nil event in my VC 's deinit seems to break the cycle but this solution is far from idea.

MemLeakBinding.zip

Originally brought this up in the PFC Slack community: https://pointfreecommunity.slack.com/archives/C04L7QT8L2Y/p1743131001044919

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

When using

UIViewController.present<Item: Identifiable>(
      item: UIBinding<Item?>,
      onDismiss: (() -> Void)? = nil,
      content: @escaping (Item) -> UIViewController
    ) -> ObserveToken

and the appropriate View controller is presented then, a few moments later dismissed. Both the View Controller and the observed object are released.

Actual behavior

Only the view controller is released. The observed object retains itself and never goes away. If it is for a View Controller that is presented and dismissed multiple times of the app lifecycle, you will end up with N instances of the observed object where N is the number of times your app presented the feature.

Steps to reproduce

Using the sample project provided:

When using the "Mem leak flow" button, I expect dismissing that presentation will result in a retain cycle on ChildFeatureViewModel. So if you present the child feature 5 times, at the end in the memory debugger you'll see 5 instances of the VM alive.
When using the "Dirty fix flow" button, it shows the same child feature but once dismissed, it gets released from memory

SwiftUI Navigation version information

2.3.0

Destination operating system

iOS 17 and iOS 18

Xcode version information

Xcode 16.0.0

Swift Compiler version information

swift-driver version: 1.115 Apple Swift version 6.0 (swiftlang-6.0.0.9.10 clang-1600.0.26.2)
Target: arm64-apple-macosx15.0
@stephencelis
Copy link
Member

@lucolivier-dumaisblais Thanks for the report! It's subtle, but the problem is the use of @UIBinding instead of @UIBindable. @UIBinding is meant for value types that need to be boxed up, and this box creates a strong reference. If you use @UIBindable to hold onto your model instead, the child is deallocated as expected.

I've opened #284 to warn against this accidental misuse.

@lucolivier-dumaisblais
Copy link
Author

@stephencelis Thanks, I applied the change and it solved my memory leak! Really appreciate the help 🙏

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

No branches or pull requests

2 participants