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

Leak memory when change root view controller #45

Closed
phuongvnc opened this issue May 14, 2018 · 15 comments
Closed

Leak memory when change root view controller #45

phuongvnc opened this issue May 14, 2018 · 15 comments

Comments

@phuongvnc
Copy link

When i changed root view controller for my app. Life time tracker show 4 leak memory in my app. I don't know it is issue when set LifetimeConfiguration(maxCount: 1, groupName: "VC") in BaseViewController

@krzysztofzablocki
Copy link
Owner

The results we surface wouldn't be affected by anything other than lifetime of your objects, sounds to me like you have retain cycle in your object

@ghost
Copy link

ghost commented Aug 14, 2018

I am facing exactly same issue, if I change RootViewController, I can see from my console and using Memory graph tool of Xcode that all my view controllers are been deallocated, while Lifetimetracker shows opposite.

@krzysztofzablocki
Copy link
Owner

@berdikhan-fave it tracks dealloc, you can try putting deinit func and breakpoint inside of it, see if its getting call and you should know if object is really deallocated

@ghost
Copy link

ghost commented Aug 14, 2018

I verified it using 3 ways:

  1. I put print("deinit") at deinit() function of viewController
  2. Xcode's Memory graph tool
  3. https://twitter.com/0xced/status/900692839557992449 (Breakpoint) very useful by the way.

@krzysztofzablocki
Copy link
Owner

and LifetimeTracker still shows it as alive and even more so as leaking? whats the count it sees of the VC?

@ghost
Copy link

ghost commented Aug 14, 2018

Count is 1/1, supposed to be 0 or ideally not even in the list of Lifetimetracker

@ghost
Copy link

ghost commented Aug 15, 2018

[UPDATE] I was able to solve it by adding Lifetimetrackable to every viewController separately, as previously I added it to my BaseViewController and it didn't work all the time. Thanks @krzysztofzablocki for support.

@krzysztofzablocki
Copy link
Owner

@berdikhan-fave so I'm curious whether that means one of them was still alive?

@isimple4
Copy link

isimple4 commented Mar 26, 2019

Sorry to comment on this closed issue, but I also run into this annoying issue.

Here is the thing. It actually is a bug due to the use of static var for lifetimeConfiguration. If you implement LifetimeTrackable in a base view controller which people would do for convenience use, all sub view controllers will shared the static lifetimeConfiguration instance since it is static.

It would be alright if you have only one vc instance for one page, but when it comes to multiple instance, say a segment with couple of controllers to switch, the configuration name would be modified to the last instanceName and pointerString . As a result, the count of instance usage will de deducted only to the last instance rather than others, so the tracker would report memory leak of these instance.

let instanceType = type(of: instance)
var configuration = configuration
configuration.instanceName = String(describing: instanceType)
configuration.pointerString = "\(Unmanaged<AnyObject>.passUnretained(instance as AnyObject).toOpaque())"

If we put the lifetimeConfiguration under every instance, it just works fine.

@krzysztofzablocki
Copy link
Owner

interesting, didn't think about it as I don't really use inheritance for things I usually track

@tschob
Copy link
Collaborator

tschob commented Mar 26, 2019

@isimple4 do you have an example code of your setup? I used LifetimeTracker in base view controllers in multiple
projects. It worked fine with some convenience logic I added. I can prepare some example code later, but also would like to understand if this is actually the problem you faced

@isimple4
Copy link

@tschob Sure. Basically I just modified bit of source codes and copied them to my code base since I really don't want to implement LifetimeTrackable for every vc. But it's not a good practice though.

I need to change static var to instance var for protocol LifetimeTrackable, and also some related codes regarding lifetimeConfiguration in LifetimeTracker.swift file. Then every sub vc would contain its own lifetimeConfiguration instance.

If you are not using multiple instances of different sub vc for one page, there should be no worries.

@isimple4
Copy link

For multiple instance scenario, codes would be something like below.

class BaseVC: LifetimeTrackable {
    static var lifetimeConfiguration = LifetimeConfiguration()
}

class SubVC1: BaseVC { }
class SubVC2: BaseVC { }

// This instance will report memory leak of SubVC1 and SubVC2.
// And SubVC3 instance would have a count of -2.
class SubVC3: BaseVC {
    let vc1 = SubVC1()
    let vc2 = SubVC2()
}

Hope it would clarify a bit.

@tschob
Copy link
Collaborator

tschob commented Mar 26, 2019

@isimple4 I see. I modified the example project to reproduce the issue you face. Once I reentered a screen with multiple child VCs, I saw the weird behaviour! [see commit 1]

I think in general a deep dive into the library would be good to see if a proper fix can be made.

But you can also easily fix the issue in your project: Just make the configuration a computed property! [see commit 2]. What I also usually do in my projects is to create computed (class) properties which allows me to override e.g. the maxCount in children without implementing LifetimeTracker in all the VCs. [see commit 3].

Does this solve your issue / question?

PS: I pushed the code to https://github.com/krzysztofzablocki/LifetimeTracker/tree/issues/%2345/example

@isimple4
Copy link

@tschob Your solution looks much better than mine. I'll give a try, thanks for that!

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