add cleanup task to widget pool

ui/browsing/router.go

@@ -15,7 +15,7 @@ type Router struct {
 	App        *backend.App
 	Controller *controller.Controller
 	Nav        NavigationHandler
-	widgetPool util.WidgetPool
+	widgetPool *util.WidgetPool
 }
 
 func NewRouter(app *backend.App, controller *controller.Controller, nav NavigationHandler) Router {
@@ -31,27 +31,27 @@ func NewRouter(app *backend.App, controller *controller.Controller, nav Navigati
 func (r Router) CreatePage(rte controller.Route) Page {
 	switch rte.Page {
 	case controller.Album:
-		return NewAlbumPage(rte.Arg, &r.App.Config.AlbumPage, &r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller)
+		return NewAlbumPage(rte.Arg, &r.App.Config.AlbumPage, r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller)
 	case controller.Albums:
-		return NewAlbumsPage(&r.App.Config.AlbumsPage, &r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager)
+		return NewAlbumsPage(&r.App.Config.AlbumsPage, r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager)
 	case controller.Artist:
-		return NewArtistPage(rte.Arg, &r.App.Config.ArtistPage, &r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller)
+		return NewArtistPage(rte.Arg, &r.App.Config.ArtistPage, r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager, r.Controller)
 	case controller.Artists:
-		return NewArtistsPage(r.Controller, &r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager)
+		return NewArtistsPage(r.Controller, r.widgetPool, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager)
 	case controller.Favorites:
-		return NewFavoritesPage(&r.App.Config.FavoritesPage, &r.widgetPool, r.Controller, r.App.ServerManager.Server, r.App.PlaybackManager, r.App.ImageManager)
+		return NewFavoritesPage(&r.App.Config.FavoritesPage, r.widgetPool, r.Controller, r.App.ServerManager.Server, r.App.PlaybackManager, r.App.ImageManager)
 	case controller.Genre:
-		return NewGenrePage(rte.Arg, &r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager)
+		return NewGenrePage(rte.Arg, r.widgetPool, r.Controller, r.App.PlaybackManager, r.App.ServerManager.Server, r.App.ImageManager)
 	case controller.Genres:
 		return NewGenresPage(r.Controller, r.App.ServerManager.Server)
 	case controller.NowPlaying:
-		return NewNowPlayingPage(rte.Arg, r.Controller, &r.widgetPool, &r.App.Config.NowPlayingPage, r.App.PlaybackManager, r.App.Player)
+		return NewNowPlayingPage(rte.Arg, r.Controller, r.widgetPool, &r.App.Config.NowPlayingPage, r.App.PlaybackManager, r.App.Player)
 	case controller.Playlist:
-		return NewPlaylistPage(rte.Arg, &r.App.Config.PlaylistPage, &r.widgetPool, r.Controller, r.App.ServerManager, r.App.PlaybackManager, r.App.ImageManager)
+		return NewPlaylistPage(rte.Arg, &r.App.Config.PlaylistPage, r.widgetPool, r.Controller, r.App.ServerManager, r.App.PlaybackManager, r.App.ImageManager)
 	case controller.Playlists:
-		return NewPlaylistsPage(r.Controller, &r.widgetPool, &r.App.Config.PlaylistsPage, r.App.ServerManager.Server)
+		return NewPlaylistsPage(r.Controller, r.widgetPool, &r.App.Config.PlaylistsPage, r.App.ServerManager.Server)
 	case controller.Tracks:
-		return NewTracksPage(r.Controller, &r.App.Config.TracksPage, &r.widgetPool, r.App.ServerManager.Server)
+		return NewTracksPage(r.Controller, &r.App.Config.TracksPage, r.widgetPool, r.App.ServerManager.Server)
 	}
 	return nil
 }

ui/util/widgetpool.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"sync"
 	"time"
 
 	"fyne.io/fyne/v2"
@@ -19,11 +20,16 @@ const (
 	numWidgetTypes
 )
 
+const (
+	multipleItemsExpiry = 2 * time.Minute
+	singleItemExpiry    = 5 * time.Minute
+)
+
 // A pool to share commonly-used widgets across pages to reduce
 // creation of new widgets and memory allocations.
-// It is not thread-safe, which is fine for its current use.
 type WidgetPool struct {
-	pool [][]pooledWidget
+	mut   sync.Mutex
+	pools [][]pooledWidget
 }
 
 type pooledWidget struct {
@@ -31,21 +37,30 @@ type pooledWidget struct {
 	releasedAt int64 // unixMillis
 }
 
-func NewWidgetPool() WidgetPool {
+func NewWidgetPool() *WidgetPool {
-	return WidgetPool{
+	p := &WidgetPool{
-		pool: make([][]pooledWidget, numWidgetTypes),
+		pools: make([][]pooledWidget, numWidgetTypes),
 	}
+	go func() {
+		t := time.NewTicker(2 * time.Minute)
+		for range t.C {
+			p.cleanUpExpiredItems()
+		}
+	}()
+	return p
 }
 
 // Obtain obtains a widget of the given type from the pool, if one exists.
 // Returns nil if there is no available widget.
 func (w *WidgetPool) Obtain(typ WidgetType) fyne.CanvasObject {
+	w.mut.Lock()
+	defer w.mut.Unlock()
 	var widget fyne.CanvasObject
-	if l := len(w.pool[typ]); l > 0 {
+	if l := len(w.pools[typ]); l > 0 {
 		i := l - 1
-		widget = w.pool[typ][i].widget
+		widget = w.pools[typ][i].widget
-		w.pool[typ][i].widget = nil
+		w.pools[typ][i].widget = nil
-		w.pool[typ] = w.pool[typ][:i]
+		w.pools[typ] = w.pools[typ][:i]
 	}
 	return widget
 }
@@ -54,8 +69,31 @@ func (w *WidgetPool) Obtain(typ WidgetType) fyne.CanvasObject {
 // The widget must not be modified by the releaser after release,
 // since it may be Obtained for a new use at any time.
 func (w *WidgetPool) Release(typ WidgetType, wid fyne.CanvasObject) {
-	w.pool[typ] = append(w.pool[typ], pooledWidget{
+	w.mut.Lock()
+	defer w.mut.Unlock()
+	w.pools[typ] = append(w.pools[typ], pooledWidget{
 		widget:     wid,
 		releasedAt: time.Now().UnixMilli(),
 	})
 }
+
+func (w *WidgetPool) cleanUpExpiredItems() {
+	w.mut.Lock()
+	defer w.mut.Unlock()
+	for widTyp, pool := range w.pools {
+		newP := make([]pooledWidget, 0, len(pool))
+		l := len(pool)
+		for _, wid := range pool {
+			timeSinceRelease := time.Since(time.UnixMilli(wid.releasedAt))
+			if l > 1 && timeSinceRelease > multipleItemsExpiry {
+				l-- // let expire if >1 item in pool and released long enough ago
+			} else if l == 1 && timeSinceRelease > singleItemExpiry {
+				l-- // let expire if last item in pool and released long enough ago
+			} else {
+				newP = append(newP, wid) // not expired
+			}
+		}
+		// re-assign non-expired items back to this widget type pool
+		w.pools[widTyp] = newP
+	}
+}

ui/util/widgetpool_test.go

@@ -0,0 +1,24 @@
+package util
+
+import (
+	"testing"
+	"time"
+)
+
+func Test_WidgetPool_CleanupExpiredItems(t *testing.T) {
+	now := time.Now()
+	threeMinAgo := now.Add(-3 * time.Minute)
+	w := &WidgetPool{
+		pools: [][]pooledWidget{
+			{
+				{releasedAt: threeMinAgo.UnixMilli()},
+				{releasedAt: threeMinAgo.UnixMilli()},
+				{releasedAt: threeMinAgo.UnixMilli()},
+			},
+		},
+	}
+	w.cleanUpExpiredItems()
+	if l := len(w.pools[0]); l != 1 {
+		t.Errorf("Expected one widget in pool after cleanup, got %d", l)
+	}
+}