1.0 alpha treeChangesQueue creates a memory leak

[levraipixel]

Hi,

As #545 doesn't seem fixed when using "resolve", I tried to jump to v1.0.0alpha0 but there seems to be a leak with treeChangesQueue : my view controller gets retained.

It doesn't seem to require usage of "resolve" to happen.
I'm using Angular 1.4.9.

Please tell me if I can help you guys by giving any more details, I would really like to be able to use "resolve" with one version or another without creating a leak anymore.

Best,

[christopherthielen]

yes; We don't technically need to retain these objects (transitionsQueue and treeChangesQueue), but rather only retain the last one. Once we create a history api, we'll need to retain the changes, but we can retain only the subset of each object necessary to implement the history api.

[levraipixel]

Oh I get it, that seems right.
I know this is an annoying question, but... do you think you will do that soon?
I can help if needed, but I've never contributed to that repo so it might be quite hard for me to get in.

[nateabele]

Sure, you could start with this discussion.

Your best bet is probably to implement it as a separate plugin. If your plugin needs to do something that the UI Router API doesn't expose, we'll start there, and we're happy to help review code/ideas if you need.

[colinmorelli]

@christopherthielen saw your merge but this issue appears to still exist on 1.0.0-beta.3 (though through a slightly different retain path of _treeChanges on the Transition rather than treeChangesQueue):

[christopherthielen]

@colinmorelli I think that your screenshot shows a $scope retaining the $resolve object, which in turn retains the $transition$ which activated the state.

The transition itself retains the entering and exiting paths, which is what you're showing. I think this is reasonably acceptable since it will be limited to the enter/exit paths for recently completed transitions. In other words, I think memory usage will not continue to grow, but will flatten out almost immediately.

If memory usage is a major concern, you can write an onSuccess hook to clear resolvables from the exiting array, but doing so may cause problems elsewhere (I'm honestly not sure).

A hook of that sort would be like:

$transitions.onSuccess({}, transition => 
    transition.treeChanges().exiting.forEach(node => node.resolvables = []))

---

I'm not saying that retaining resolve data from an exited state is proper and intended, but I am saying there may be other consequences to not retaining it. I'm honestly not sure.

[colinmorelli]

@christopherthielen I haven't tested the upper bound by any means, but just by playing around while investigating this issue, it wasn't clear where it would stop (it was still holding on to objects from 5 transitions ago). There's also a deeper retain path that can be seen here:

I don't think it's the end of the world for a transition to retain its entering/exiting paths, but I would be curious as to why it would need continued reference to those objects after the transition is complete. It seems like part of the transition should be clearing references to the transition when it's done, though admittedly I haven't looked deep enough into the 1.x rewrite to know if there's justification for the current behavior.

[christopherthielen]

I don't think it's the end of the world for a transition to retain its entering/exiting paths, but I would be curious as to why it would need continued reference to those objects after the transition is complete.

Agreed, and like I said earlier, I'm not sure that retaining those references is actually necessary, only that there may be something that expects them.

• Note that those retained resolvables references definitely need to be retained.
• The resolves in the from path should also be in the retained or exited path.

I'm certainly open to cleaning up the exited resolve data. It just makes sense to clear those references, as you say. I just want to be sure it doesn't have any bad side effects.

[colinmorelli]

Note that those retained resolvables references definitely need to be retained.

Understood on this point. But it looks like one of the things that ends up being pushed into that retained array is the previous instance of the PathNode that was there. Ultimately, as you continue entering and exiting the state, you'll end up with a retain graph that looks like:

It appears to continue growing with each entry into the route. Unlike simply cleaning up the $transition$ variable on the scope, this seems to indicate potentially a deeper problem? (Although cleaning the previous $transition$ from the scope should still solve this problem).

[jbarrus]

@colinmorelli did you find a solution to this? I'm not positive this is my problem, but the # of DOM nodes is increasing with every transition.

[christopherthielen]

@jbarrus there's an issue related to DOM nodes increasing. I've not been able to reproduce it. A simple reproduction would be really helpful. #545 (comment)

[jbarrus]

This ended up being my biggest issue - https://bugs.chromium.org/p/chromium/issues/detail?id=707544 Installing chromium 56, my DOM node issue goes away.

Though, I am still interested to know if it's possible to reduce the ui-router footprint as mentioned earlier.