limit concurrent image fetches from server

backend/imagemanager.go

@@ -21,13 +21,13 @@ import (
 	"github.com/google/uuid"
 )
 
-const CachedImageValidTime = 24 * time.Hour
-
 const (
 	coverArtThumbnailSize = 300
+	cachedImageValidTime  = 24 * time.Hour
 	fullSizeCoverExpires  = 5 * time.Minute
 
-	defaultDiskCacheSizeBytes = 50 * 1_048_576
+	maxConcurrentServerFetches = 5
+	defaultDiskCacheSizeBytes  = 50 * 1_048_576
 )
 
 // The ImageManager is responsible for retrieving and serving images to the UI layer.
@@ -44,8 +44,11 @@ type ImageManager struct {
 
 	maxOnDiskCacheSizeBytes    int64
 	filesWrittenSinceLastPrune bool
+
+	serverFetchSema chan interface{}
 }
 
+// NewImageManager returns a new ImageManager.
 func NewImageManager(ctx context.Context, s *ServerManager, baseCacheDir string) *ImageManager {
 	if err := configdir.MakePath(baseCacheDir); err != nil {
 		log.Println("failed to create album cover cache dir")
@@ -60,6 +63,7 @@ func NewImageManager(ctx context.Context, s *ServerManager, baseCacheDir string)
 			DefaultTTL: 1 * time.Minute,
 		},
 		maxOnDiskCacheSizeBytes: defaultDiskCacheSizeBytes,
+		serverFetchSema:         make(chan interface{}, maxConcurrentServerFetches),
 	}
 	s.OnLogout(func() {
 		i.thumbnailCache.Clear()
@@ -73,10 +77,14 @@ func NewImageManager(ctx context.Context, s *ServerManager, baseCacheDir string)
 	return i
 }
 
+// SetMaxOnDiskCacheSizeBytes sets the maximum size of the on-disc cover thumbnail cache.
+// A periodic clean task will delete least recently accessed images to maintain the size limit.
 func (i *ImageManager) SetMaxOnDiskCacheSizeBytes(size int64) {
 	i.maxOnDiskCacheSizeBytes = size
 }
 
+// GetCoverThumbnailFromCache returns the cover thumbnail for the given ID if it exists
+// in the in-memory cache. Returns quickly, safe to call in UI threads.
 func (i *ImageManager) GetCoverThumbnailFromCache(coverID string) (image.Image, bool) {
 	img, err := i.thumbnailCache.GetExtendTTL(coverID, i.thumbnailCache.DefaultTTL)
 	if err == nil && img != nil {
@@ -85,21 +93,44 @@ func (i *ImageManager) GetCoverThumbnailFromCache(coverID string) (image.Image,
 	return nil, false
 }
 
+// GetCoverThumbnailAsync asynchronously fetches the cover image for the given ID,
+// and invokes the callback on completion. It returns a context.CancelFunc which can be used to
+// cancel the fetch. The callback will not be invoked if the fetch is cancelled before completion.
+// The cancel func must be invoked to avoid resource leaks. Use GetCoverThumbnail if cancellation is not needed.
+func (i *ImageManager) GetCoverThumbnailAsync(coverID string, cb func(image.Image, error)) context.CancelFunc {
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		if im, ok := i.GetCoverThumbnailFromCache(coverID); ok {
+			if ctx.Err() == nil {
+				cb(im, nil)
+			}
+		} else {
+			i.fetchAndCacheCoverFromDiskOrServer(ctx, coverID, i.thumbnailCache.DefaultTTL, cb)
+		}
+	}()
+	return cancel
+}
+
+// GetCoverThumbnail is a synchronous, blocking function to fetch the image for a given coverID.
+// Like most ImageManager calls, it should usually be called in a goroutine to not block UI loading.
 func (i *ImageManager) GetCoverThumbnail(coverID string) (image.Image, error) {
 	if im, ok := i.GetCoverThumbnailFromCache(coverID); ok {
 		return im, nil
 	}
-	return i.fetchAndCacheCoverFromDiskOrServer(coverID, i.thumbnailCache.DefaultTTL)
+	return i.fetchAndCacheCoverFromDiskOrServer(context.Background(), coverID, i.thumbnailCache.DefaultTTL, nil)
 }
 
+// GetCoverThumbnailWithTTL fetches the cover for the given coverID and updates the TTL
+// in the in-memory image cache. It blocks until the image fetch is complete.
 func (i *ImageManager) GetCoverThumbnailWithTTL(coverID string, ttl time.Duration) (image.Image, error) {
-	// in-memory cache
 	if img, err := i.thumbnailCache.GetWithNewTTL(coverID, ttl); err == nil {
 		return img, nil
 	}
-	return i.fetchAndCacheCoverFromDiskOrServer(coverID, ttl)
+	return i.fetchAndCacheCoverFromDiskOrServer(context.Background(), coverID, ttl, nil)
 }
 
+// GetFullSizeCoverArt fetches the full size cover image for the given coverID.
+// It blocks until the fetch is complete.
 func (i *ImageManager) GetFullSizeCoverArt(coverID string) (image.Image, error) {
 	if i.cachedFullSizeCoverID == coverID {
 		i.cachedFullSizeCoverAccessedAt = time.Now().UnixMilli()
@@ -108,7 +139,10 @@ func (i *ImageManager) GetFullSizeCoverArt(coverID string) (image.Image, error)
 	if i.s.Server == nil {
 		return nil, errors.New("logged out")
 	}
+
+	i.serverFetchSema <- struct{}{} // acquire
 	im, err := i.s.Server.GetCoverArt(coverID, 0)
+	<-i.serverFetchSema // release
 	if err != nil {
 		return nil, err
 	}
@@ -118,6 +152,7 @@ func (i *ImageManager) GetFullSizeCoverArt(coverID string) (image.Image, error)
 	return im, nil
 }
 
+// GetCoverArtURL returns the URL for the locally cached cover thumbnail, if it exists.
 func (i *ImageManager) GetCoverArtUrl(coverID string) (string, error) {
 	path := i.filePathForCover(coverID)
 	if _, err := os.Stat(path); err == nil {
@@ -128,10 +163,13 @@ func (i *ImageManager) GetCoverArtUrl(coverID string) (string, error) {
 	return "", errors.New("cover not found")
 }
 
+// GetCachedArtistImage returns the artist image for the given artistID from the on-disc cache, if it exists.
 func (i *ImageManager) GetCachedArtistImage(artistID string) (image.Image, bool) {
 	return i.loadLocalImage(i.filePathForArtistImage(artistID))
 }
 
+// FetchAndCacheArtistImage fetches the artist image for the given artistID from the server,
+// caching it locally if the fetch succeeds. Blocks until fetch is completed.
 func (i *ImageManager) FetchAndCacheArtistImage(artistID string, imgURL string) (image.Image, error) {
 	im, err := i.fetchRemoteArtistImage(imgURL)
 	if err != nil {
@@ -141,9 +179,10 @@ func (i *ImageManager) FetchAndCacheArtistImage(artistID string, imgURL string)
 	return im, nil
 }
 
+// RefreshCachedArtistImageIfExpired re-fetches the artist image from the server if expired.
 func (i *ImageManager) RefreshCachedArtistImageIfExpired(artistID string, imgURL string) error {
 	stat, err := os.Stat(i.filePathForArtistImage(artistID))
-	if err == nil && time.Since(stat.ModTime()) > CachedImageValidTime {
+	if err == nil && time.Since(stat.ModTime()) > cachedImageValidTime {
 		_, err = i.FetchAndCacheArtistImage(artistID, imgURL)
 	}
 	return err
@@ -167,7 +206,10 @@ func (i *ImageManager) ensureArtistCoverCacheDir() string {
 }
 
 func (i *ImageManager) fetchRemoteArtistImage(url string) (image.Image, error) {
+	i.serverFetchSema <- struct{}{} // acquire
 	res, err := fyne.LoadResourceFromURLString(url)
+	<-i.serverFetchSema // release
+
 	if err == nil {
 		im, _, err := image.Decode(bytes.NewReader(res.Content()))
 		if err == nil {
@@ -178,41 +220,56 @@ func (i *ImageManager) fetchRemoteArtistImage(url string) (image.Image, error) {
 	return nil, err
 }
 
-func (i *ImageManager) fetchAndCacheCoverFromDiskOrServer(coverID string, ttl time.Duration) (image.Image, error) {
+func (i *ImageManager) fetchAndCacheCoverFromDiskOrServer(ctx context.Context, coverID string, ttl time.Duration, cb func(image.Image, error)) (image.Image, error) {
 	// on disc cache
-	path := i.filePathForCover(coverID)
 	if i.ensureCoverCacheDir() != "" {
+		path := i.filePathForCover(coverID)
 		if s, err := os.Stat(path); err == nil {
 			go i.checkRefreshLocalCover(s, coverID, ttl)
 			if img, ok := i.loadLocalImage(path); ok {
 				i.thumbnailCache.SetWithTTL(coverID, img, ttl)
+				if ctx.Err() == nil && cb != nil {
+					cb(img, nil)
+				}
 				return img, nil
 			}
 		}
 	}
 
-	return i.fetchAndCacheCoverFromServer(coverID, ttl)
+	// fetch from server
+	return i.fetchAndCacheCoverFromServer(ctx, coverID, ttl, cb)
 }
 
-func (i *ImageManager) fetchAndCacheCoverFromServer(coverID string, ttl time.Duration) (image.Image, error) {
+func (i *ImageManager) fetchAndCacheCoverFromServer(ctx context.Context, coverID string, ttl time.Duration, cb func(image.Image, error)) (image.Image, error) {
 	if i.s.Server == nil {
-		return nil, errors.New("logged out")
+		err := errors.New("logged out")
-	}
+		if ctx.Err() == nil && cb != nil {
-	img, err := i.s.Server.GetCoverArt(coverID, coverArtThumbnailSize)
+			cb(nil, err)
-	if err != nil {
+		}
 		return nil, err
 	}
-	if i.ensureCoverCacheDir() != "" {
+	select {
-		path := i.filePathForCover(coverID)
+	case <-ctx.Done():
-		_ = i.writeJpeg(img, path)
+		return nil, context.Canceled
+	case i.serverFetchSema <- struct{}{}: // acquire
+		img, err := i.s.Server.GetCoverArt(coverID, coverArtThumbnailSize)
+		<-i.serverFetchSema // release
+		if err == nil {
+			if i.ensureCoverCacheDir() != "" {
+				_ = i.writeJpeg(img, i.filePathForCover(coverID))
+			}
+			i.thumbnailCache.SetWithTTL(coverID, img, ttl)
+		}
+		if ctx.Err() == nil && cb != nil {
+			cb(img, err)
+		}
+		return img, err
 	}
-	i.thumbnailCache.SetWithTTL(coverID, img, ttl)
-	return img, nil
 }
 
 func (i *ImageManager) checkRefreshLocalCover(stat os.FileInfo, coverID string, ttl time.Duration) {
-	if time.Since(stat.ModTime()) > CachedImageValidTime {
+	if time.Since(stat.ModTime()) > cachedImageValidTime {
-		i.fetchAndCacheCoverFromServer(coverID, ttl)
+		i.fetchAndCacheCoverFromServer(context.Background(), coverID, ttl, nil)
 	}
 }
 

ui/widgets/gridview.go

@@ -40,7 +40,7 @@ func (b *BatchingIterator) NextN(n int) []*mediaprovider.Album {
 
 type ImageFetcher interface {
 	GetCoverThumbnailFromCache(string) (image.Image, bool)
-	GetCoverThumbnail(string) (image.Image, error)
+	GetCoverThumbnailAsync(string, func(image.Image, error)) context.CancelFunc
 }
 
 type GridViewIterator interface {
@@ -126,7 +126,7 @@ func NewGridView(iter GridViewIterator, fetch ImageFetcher, placeholder fyne.Res
 	g.createGridWrap()
 
 	// fetch initial items
-	g.fetchMoreItems(36)
+	g.checkFetchMoreItems(36)
 	return g
 }
 
@@ -148,37 +148,28 @@ func NewGridViewFromState(state *GridViewState) *GridView {
 }
 
 func (g *GridView) Clear() {
-	if g.fetchCancel != nil {
-		g.fetchCancel()
-		g.fetchCancel = nil
-	}
 	g.stateMutex.Lock()
 	defer g.stateMutex.Unlock()
+	g.cancelFetch()
 	g.items = nil
 	g.done = true
 }
 
 func (g *GridView) Reset(iter GridViewIterator) {
-	if g.fetchCancel != nil {
-		g.fetchCancel()
-		g.fetchCancel = nil
-	}
 	g.stateMutex.Lock()
+	g.cancelFetch()
 	g.items = nil
 	g.done = false
 	g.highestShown = 0
 	g.iter = iter
 	g.stateMutex.Unlock()
-	g.fetchMoreItems(36)
+	g.checkFetchMoreItems(36)
 	g.Refresh()
 }
 
 func (g *GridView) ResetFromState(state *GridViewState) {
-	if g.fetchCancel != nil {
-		g.fetchCancel()
-		g.fetchCancel = nil
-	}
 	g.stateMutex.Lock()
+	g.cancelFetch()
 	g.GridViewState = *state
 	g.stateMutex.Unlock()
 	g.grid.Refresh()
@@ -187,11 +178,8 @@ func (g *GridView) ResetFromState(state *GridViewState) {
 }
 
 func (g *GridView) ResetFixed(items []GridViewItemModel) {
-	if g.fetchCancel != nil {
-		g.fetchCancel()
-		g.fetchCancel = nil
-	}
 	g.stateMutex.Lock()
+	g.cancelFetch()
 	g.items = items
 	g.done = true
 	g.highestShown = 0
@@ -259,32 +247,23 @@ func (g *GridView) doUpdateItemCard(itemIdx int, card *GridViewItem) {
 	}
 	card.Update(item)
 	card.PrevID = item.ID
-	// cancel any previous image fetch
+	// cancel any previous image fetch (no issues with possible double-invocations)
 	if card.ImgLoadCancel != nil {
 		card.ImgLoadCancel()
-		card.ImgLoadCancel = nil
 	}
 	if item.CoverArtID != "" {
 		if img, ok := g.imageFetcher.GetCoverThumbnailFromCache(item.CoverArtID); ok {
 			card.Cover.SetImage(img)
 		} else {
 			card.Cover.SetImage(nil)
-			// asynchronously fetch cover image
+			card.ImgLoadCancel = g.imageFetcher.GetCoverThumbnailAsync(item.CoverArtID, func(i image.Image, err error) {
-			ctx, cancel := context.WithCancel(context.Background())
+				if err == nil {
-			card.ImgLoadCancel = cancel
+					card.Cover.SetImage(i)
-			go func(ctx context.Context) {
+				} else {
-				i, err := g.imageFetcher.GetCoverThumbnail(item.CoverArtID)
+					log.Printf("error fetching image: %s", err.Error())
-				select {
-				case <-ctx.Done():
-					return
-				default:
-					if err == nil {
-						card.Cover.SetImage(i)
-					} else {
-						log.Printf("error fetching image: %s", err.Error())
-					}
 				}
-			}(ctx)
+				card.ImgLoadCancel() // done. release resources associated with cancel channel
+			})
 		}
 	} else {
 		// use the placeholder for an item that has no cover art ID
@@ -292,8 +271,8 @@ func (g *GridView) doUpdateItemCard(itemIdx int, card *GridViewItem) {
 	}
 
 	// if user has scrolled near the bottom, fetch more
-	if !g.done && g.fetchCancel == nil && itemIdx > g.lenItems()-10 {
+	if itemIdx > g.lenItems()-10 {
-		g.fetchMoreItems(20)
+		g.checkFetchMoreItems(20)
 	}
 }
 
@@ -303,8 +282,14 @@ func (g *GridView) lenItems() int {
 	return len(g.items)
 }
 
-// fetches at least count more items
+// fetches at least count more items if fetch not in progress and not done
-func (g *GridView) fetchMoreItems(count int) {
+// acquires stateMutex for atomicity
+func (g *GridView) checkFetchMoreItems(count int) {
+	g.stateMutex.Lock()
+	defer g.stateMutex.Unlock()
+	if g.done || g.fetchCancel != nil {
+		return // done, or fetch already in progress
+	}
 	if g.iter == nil {
 		g.done = true
 		return
@@ -335,10 +320,21 @@ func (g *GridView) fetchMoreItems(count int) {
 				}
 			}
 		}
-		g.fetchCancel = nil
+		// call cancelfunc to release Context resources
+		g.stateMutex.Lock()
+		g.cancelFetch()
+		g.stateMutex.Unlock()
 	}()
 }
 
+// must be called with stateMutex locked for writing
+func (g *GridView) cancelFetch() {
+	if g.fetchCancel != nil {
+		g.fetchCancel()
+		g.fetchCancel = nil
+	}
+}
+
 func (g *GridView) showContextMenu(card *GridViewItem, pos fyne.Position) {
 	g.menuGridViewItemId = card.ItemID()
 	if g.menu == nil {