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

Does introspect cause memory leak? #161

Closed
okmyself opened this issue Aug 26, 2022 · 10 comments
Closed

Does introspect cause memory leak? #161

okmyself opened this issue Aug 26, 2022 · 10 comments

Comments

@okmyself
Copy link
Contributor

okmyself commented Aug 26, 2022

@crayment When I use introspect in a struct containing a viewModel, the deinit function in the viewModel never executes.
Version: Master branch on github (2022-8-25), I downloaded this to fix #140, but found this issue instead.

@okmyself okmyself reopened this Aug 26, 2022
@okmyself
Copy link
Contributor Author

okmyself commented Aug 26, 2022

After modifying to the following code, the memory leak problem seems to be solved.

#if canImport(UIKit)
import UIKit
import SwiftUI

/// Introspection UIView that is inserted alongside the target view.
@available(iOS 13.0, *)
public class IntrospectionUIView: UIView {
    
    var moveToWindowHandler: (() -> Void)?
    
    required init() {
        super.init(frame: .zero)
        isHidden = true
        isUserInteractionEnabled = false
    }
    
    @available(*, unavailable)
    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
    
    override public func didMoveToWindow() {
        super.didMoveToWindow()
        moveToWindowHandler?()
    }
}

/// Introspection View that is injected into the UIKit hierarchy alongside the target view.
/// After `updateUIView` is called, it calls `selector` to find the target view, then `customize` when the target view is found.
@available(iOS 13.0, tvOS 13.0, macOS 10.15.0, *)
public struct UIKitIntrospectionView<TargetViewType: UIView>: UIViewRepresentable {
    
    /// Method that introspects the view hierarchy to find the target view.
    /// First argument is the introspection view itself, which is contained in a view host alongside the target view.
    let selector: (IntrospectionUIView) -> TargetViewType?
    
    /// User-provided customization method for the target view.
    let customize: (TargetViewType) -> Void
    
    public init(
        selector: @escaping (IntrospectionUIView) -> TargetViewType?,
        customize: @escaping (TargetViewType) -> Void
    ) {
        self.selector = selector
        self.customize = customize
    }
    
    public func makeUIView(context: UIViewRepresentableContext<UIKitIntrospectionView>) -> IntrospectionUIView {
        let view = IntrospectionUIView()
        view.accessibilityLabel = "IntrospectionUIView<\(TargetViewType.self)>"
        view.moveToWindowHandler = {
            DispatchQueue.main.async {
                guard let targetView = selector(view) else {
                    return
                }
                customize(targetView)
            }
        }
        return view
    }

    /// When `updateUiView` is called after creating the Introspection view, it is not yet in the UIKit hierarchy.
    /// At this point, `introspectionView.superview.superview` is nil and we can't access the target UIKit view.
    /// To workaround this, we wait until the runloop is done inserting the introspection view in the hierarchy, then run the selector.
    /// Finding the target view fails silently if the selector yield no result. This happens when `updateUIView`
    /// gets called when the introspection view gets removed from the hierarchy.
    public func updateUIView(
        _ uiView: IntrospectionUIView,
        context: UIViewRepresentableContext<UIKitIntrospectionView>
    ) { }
    
    public static func dismantleUIView(_ uiView: IntrospectionUIView, coordinator: ()) {
        uiView.moveToWindowHandler = nil
    }
}
#endif

@crayment
Copy link
Contributor

Thanks for reporting that! If you could open a pull request with that change I would put my support behind it and pull it into my fork. This is not my repo though so I would not be able to merge it here.

@okmyself
Copy link
Contributor Author

Thank you. I will try it.

@okmyself
Copy link
Contributor Author

okmyself commented Sep 8, 2022

Thanks for reporting that! If you could open a pull request with that change I would put my support behind it and pull it into my fork. This is not my repo though so I would not be able to merge it here.

@crayment I have completed my pull requests. Do I need to do anything else?

@okmyself okmyself closed this as completed Sep 9, 2022
@crayment
Copy link
Contributor

crayment commented Sep 9, 2022

Thanks or doing that! That should be all - but in my experience in might be a while before it gets reviewed/merged unfortunately.

@plaps153
Copy link

This really works. It solved my problem and should be merged!

@davdroman
Copy link
Collaborator

davdroman commented Feb 11, 2023

@okmyself @crayment is this still a problem? I'm happy to reopen #163 and get it merged if this issue is still ongoing.

@davdroman
Copy link
Collaborator

@okmyself please comment via the GitHub web interface as GitHub does a terrible job at rendering email responses.

@okmyself
Copy link
Contributor Author

okmyself commented Feb 11, 2023

@okmyself please comment via the GitHub web interface as GitHub does a terrible job at rendering email responses.

@davdroman This issue was introduced by bug fix #153. The version displayed on Github still has issues, but the current official version does not have this problem. I have also discovered another problem: In some cases, UIView does not call moveToWindowHandler, so the moveToWindowHandler call in updateUIView must be preserved.

UIKitIntrospectionView.swift

#if canImport(UIKit)
import UIKit
import SwiftUI

/// Introspection UIView that is inserted alongside the target view.
@available(iOS 13.0, *)
public class IntrospectionUIView: UIView {
    
    var moveToWindowHandler: (() -> Void)?
    
    required init() {
        super.init(frame: .zero)
        isHidden = true
        isUserInteractionEnabled = false
    }
    
    @available(*, unavailable)
    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
    
    override public func didMoveToWindow() {
        super.didMoveToWindow()
        moveToWindowHandler?()
    }
}

/// Introspection View that is injected into the UIKit hierarchy alongside the target view.
/// After `updateUIView` is called, it calls `selector` to find the target view, then `customize` when the target view is found.
@available(iOS 13.0, tvOS 13.0, macOS 10.15.0, *)
public struct UIKitIntrospectionView<TargetViewType: UIView>: UIViewRepresentable {
    
    /// Method that introspects the view hierarchy to find the target view.
    /// First argument is the introspection view itself, which is contained in a view host alongside the target view.
    let selector: (IntrospectionUIView) -> TargetViewType?
    
    /// User-provided customization method for the target view.
    let customize: (TargetViewType) -> Void
    
    public init(
        selector: @escaping (IntrospectionUIView) -> TargetViewType?,
        customize: @escaping (TargetViewType) -> Void
    ) {
        self.selector = selector
        self.customize = customize
    }
    
    public func makeUIView(context: UIViewRepresentableContext<UIKitIntrospectionView>) -> IntrospectionUIView {
        let view = IntrospectionUIView()
        view.accessibilityLabel = "IntrospectionUIView<\(TargetViewType.self)>"
        view.moveToWindowHandler = {
            DispatchQueue.main.async {
                guard let targetView = selector(view) else {
                    return
                }
                customize(targetView)
            }
        }
        return view
    }

    /// When `updateUiView` is called after creating the Introspection view, it is not yet in the UIKit hierarchy.
    /// At this point, `introspectionView.superview.superview` is nil and we can't access the target UIKit view.
    /// To workaround this, we wait until the runloop is done inserting the introspection view in the hierarchy, then run the selector.
    /// Finding the target view fails silently if the selector yield no result. This happens when `updateUIView`
    /// gets called when the introspection view gets removed from the hierarchy.
    public func updateUIView(
        _ uiView: IntrospectionUIView,
        context: UIViewRepresentableContext<UIKitIntrospectionView>
    ) {
        uiView.moveToWindowHandler?()
    }
    
    /// Fix memory leak issue
    public static func dismantleUIView(_ uiView: IntrospectionUIView, coordinator: ()) {
        uiView.moveToWindowHandler = nil
    }
}
#endif

@davdroman
Copy link
Collaborator

davdroman commented Feb 13, 2023

@okmyself okay I understand. Would you be willing to put forth a PR? If so I'll merge it right away. Otherwise I can do it when I get some spare time, but I'd appreciate if you could do it as I have lots of other issues to go through on this repo. Regardless: thank you for your response and sample code!

okmyself added a commit to okmyself/SwiftUI-Introspect that referenced this issue Feb 14, 2023
davdroman added a commit that referenced this issue Feb 15, 2023
Co-authored-by: David Roman <2538074+davdroman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants