Merge pull request #1956 from fpabl0/fix/anim-memory-leak

Fixes animation runner memory leak

internal/animation/animation.go

@@ -1,6 +1,7 @@
 package animation
 
 import (
+	"sync"
 	"time"
 
 	"fyne.io/fyne/v2"
@@ -13,6 +14,9 @@ type anim struct {
 	reverse     bool
 	start       time.Time
 	total       int64
+
+	mu      sync.RWMutex
+	stopped bool
 }
 
 func newAnim(a *fyne.Animation) *anim {
@@ -21,3 +25,16 @@ func newAnim(a *fyne.Animation) *anim {
 	animate.repeatsLeft = a.RepeatCount
 	return animate
 }
+
+func (a *anim) setStopped() {
+	a.mu.Lock()
+	a.stopped = true
+	a.mu.Unlock()
+}
+
+func (a *anim) isStopped() bool {
+	a.mu.RLock()
+	ret := a.stopped
+	a.mu.RUnlock()
+	return ret
+}

internal/animation/animation_test.go

@@ -3,6 +3,7 @@
 package animation
 
 import (
+	"sync"
 	"testing"
 	"time"
 
@@ -48,3 +49,46 @@ func TestGLDriver_StopAnimation(t *testing.T) {
 	run.Stop(a)
 	assert.Zero(t, len(run.animations))
 }
+
+func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) {
+	var wg sync.WaitGroup
+	run := &Runner{}
+
+	// stopping an animation immediately after start, should be effectively removed
+	// from the internal animation list (first one is added directly to animation list)
+	a := &fyne.Animation{
+		Duration: time.Second,
+		Tick:     func(f float32) {},
+	}
+	run.Start(a)
+	run.Stop(a)
+
+	// stopping animation inside tick function
+	for i := 0; i < 10; i++ {
+		wg.Add(1)
+		var b *fyne.Animation
+		b = &fyne.Animation{
+			Duration: time.Second,
+			Tick: func(d float32) {
+				run.Stop(b)
+				wg.Done()
+			}}
+		run.Start(b)
+	}
+
+	// Similar to first part, but in this time this animation should be added and then removed
+	// from pendingAnimation slice.
+	c := &fyne.Animation{
+		Duration: time.Second,
+		Tick:     func(f float32) {},
+	}
+	run.Start(c)
+	run.Stop(c)
+
+	wg.Wait()
+	// animations stopped inside tick are really stopped in the next runner cycle
+	time.Sleep(time.Second/60 + 100*time.Millisecond)
+	run.animationMutex.RLock()
+	assert.Zero(t, len(run.animations))
+	run.animationMutex.RUnlock()
+}

internal/animation/runner.go

@@ -9,34 +9,56 @@ import (
 
 // Runner is the main driver for animations package
 type Runner struct {
-	animationMutex sync.RWMutex
-	animations     []*anim
+	animationMutex    sync.RWMutex
+	animations        []*anim
+	pendingAnimations []*anim
+
+	runnerStarted bool
 }
 
 // Start will register the passed application and initiate its ticking.
 func (r *Runner) Start(a *fyne.Animation) {
 	r.animationMutex.Lock()
 	defer r.animationMutex.Unlock()
-	wasStopped := len(r.animations) == 0
 
-	r.animations = append(r.animations, newAnim(a))
-	if wasStopped {
+	if !r.runnerStarted {
+		r.runnerStarted = true
+		r.animations = append(r.animations, newAnim(a))
 		r.runAnimations()
+	} else {
+		r.pendingAnimations = append(r.pendingAnimations, newAnim(a))
 	}
 }
 
 // Stop causes an animation to stop ticking (if it was still running) and removes it from the runner.
 func (r *Runner) Stop(a *fyne.Animation) {
 	r.animationMutex.Lock()
 	defer r.animationMutex.Unlock()
-	oldList := r.animations
-	var newList []*anim
-	for _, item := range oldList {
+
+	newList := make([]*anim, 0, len(r.animations))
+	stopped := false
+	for _, item := range r.animations {
 		if item.a != a {
 			newList = append(newList, item)
+		} else {
+			item.setStopped()
+			stopped = true
 		}
 	}
 	r.animations = newList
+	if stopped {
+		return
+	}
+
+	newList = make([]*anim, 0, len(r.pendingAnimations))
+	for _, item := range r.pendingAnimations {
+		if item.a != a {
+			newList = append(newList, item)
+		} else {
+			item.setStopped()
+		}
+	}
+	r.pendingAnimations = newList
 }
 
 func (r *Runner) runAnimations() {
@@ -47,19 +69,22 @@ func (r *Runner) runAnimations() {
 			<-draw.C
 			r.animationMutex.Lock()
 			oldList := r.animations
-			r.animations = nil // clear the list so we can append any new ones after processing
 			r.animationMutex.Unlock()
-			var newList []*anim
+			newList := make([]*anim, 0, len(oldList))
 			for _, a := range oldList {
-				if r.tickAnimation(a) {
+				if !a.isStopped() && r.tickAnimation(a) {
 					newList = append(newList, a)
 				}
 			}
 			r.animationMutex.Lock()
-			r.animations = append(newList, r.animations...)
+			r.animations = append(newList, r.pendingAnimations...)
+			r.pendingAnimations = nil
 			done = len(r.animations) == 0
 			r.animationMutex.Unlock()
 		}
+		r.animationMutex.Lock()
+		r.runnerStarted = false
+		r.animationMutex.Unlock()
 		draw.Stop()
 	}()
 }
@@ -90,6 +115,7 @@ func (r *Runner) tickAnimation(a *anim) bool {
 
 		a.start = time.Now()
 		a.end = a.start.Add(a.a.Duration)
+		return true
 	}
 
 	delta := time.Since(a.start).Nanoseconds() / 1000000 // TODO change this to Milliseconds() when we drop Go 1.12