CRITICAL: MemoryLeak : Kinetic doesn't release target on complete, prevents deinit about kinetic

[markuswinkler]

hmmm, there is still an issue.
I have a timeline animation (with self) and also another single animation (also with self)

timeline.add(Kinetic.fromTo(self, duration: 0.5, from:[.Translate(0,bounds.height)], to:[.Translate(0,0)]).ease(Easing.outSine),position: 0)

I made a function to be called before the view disappears, to call timeline.kill().
But that's not enough, I also have to call Kinetic.killTweensOf(self) to really free the reference.

[u10int]

The engine does store a strong reference to each target so it can cache Tween instances associated with it for returning later via getTweensOf and killTweensOf. There could be two solutions: your option where I add a killOnCompletion() method similar to CAAnimation's removedOnCompletion, or always remove them from the cache once all tween animations associated with that object have completed, which may be the cleanest solution.

[markuswinkler]

I prefer the second one, too. I guess the vast majority of calls are one offs.
One concern remains though, how would the engine handle it if the Tween is part of a timeline?
Maybe there is keepReference() switch that's automatically added instead of the killOnCompletion() approach.

Or if someone stores the return value and tries to trigger the play handle after the animation has finished? I have no problem if that's explicitly not possible as long as it is mentioned in the docs.

[u10int]

I opted for the automatic approach for removing tweens from the cache when they have completed their animation, which is in this commit if you want to review: 5ed0e31

I verified that the tweens are removed from the internal cache by calling Kinetic.getTweensOf(target) multiple times during a timeline animation which should be nil when the animation completes and it's the only animator for the target. If the animation is played afterwards, the tweens are re-added back into the cache and removed upon completion again whenever kill() is called.

If the tween is part of a timeline, the timeline instance always retains the child tweens, so there's no need to manually store a reference to a tween within a timeline. However, if you want to be able to replay a single Tween instance that's not part of a Timeline, a strong reference will be needed since the object will be deallocated when removed from the internal cache when completed.

[markuswinkler]

Great! Good solve!
Just to be safe, if I call timeline.kill() all elements get killed of it as well, right?

[u10int]

@markuswinkler Can you provide more code on how you're implementing this animation where you're noticing the issue? Looks like you're adding the animation code directly within a UIView subclass (self is used when adding tweens).

I did a test locally trying to reproduce how you have your animation setup and didn't notice any memory leaks. Here's my simple UIView subclass where the animation is setup:

class TestView: UIView {
    var timeline = Timeline()

    func animate() {
        timeline.add(Kinetic.fromTo(self, duration: 0.5, from:[.Translate(0, bounds.height)], to:[.Translate(0,0)]).ease(Easing.outSine),position: 0)
        timeline.play()
    }

    func reset() {
        timeline.kill()
    }
}

And the simple view lifecycle methods in the controller, where testView is a strong reference property:

override func viewDidLoad() {
    super.viewDidLoad()

    testView = TestView(frame: CGRect(origin: CGPoint(x: 100, y: 200), size: CGSize(width: 100, height: 100)))
    testView.backgroundColor = UIColor.greenColor()
    view.addSubview(testView)
}

override func viewDidAppear(animated: Bool) {
    super.viewDidAppear(animated)
    testView.animate()
}

override func viewDidDisappear(animated: Bool) {
    super.viewDidDisappear(animated)
    testView.reset()
}

[markuswinkler]

Just ran your test, can confirm it works now.
You don't even have to call .kill anymore.
If I add the following code to animate in TestView:

        timeline.onComplete { [weak self](Animation) in
            self?.removeFromSuperview()
        }

and

    deinit {
        print("DEINIT: testview")
    }

deinit is called when the animation finishes.

When I posted the bug initially deinit wasn't called after completion.
Got probably solved with one of the other optimizations you put in.

[u10int]

Sweet, I like those kind of issues. :)