From 9a8dde650357924fc10dc2a48794b0a93841dc63 Mon Sep 17 00:00:00 2001
From: Hajime Hoshi <hajimehoshi@gmail.com>
Date: Mon, 29 Jan 2024 20:38:17 +0900
Subject: [PATCH] internal/atlas: bug fix: a finalizer was never called

As the finalizer function had a reference to the target image, the
image's reference count never became 0. Then, the image was never
finalized.

This change fixes this issue by using a member function pointer instead
of an anonymous function.

Closes #2897
---
 internal/atlas/export_test.go |  6 ++++++
 internal/atlas/image.go       | 18 ++++++++++--------
 internal/atlas/image_test.go  | 13 +++++++++++++
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/internal/atlas/export_test.go b/internal/atlas/export_test.go
index 0cc3b071a..dc0a00bf5 100644
--- a/internal/atlas/export_test.go
+++ b/internal/atlas/export_test.go
@@ -67,3 +67,9 @@ func (i *Image) EnsureIsolatedFromSourceForTesting(backends []*backend) {
 var FlushDeferredForTesting = flushDeferred
 
 var FloorPowerOf2 = floorPowerOf2
+
+func DeferredFuncCountForTesting() int {
+	deferredM.Lock()
+	defer deferredM.Unlock()
+	return len(deferred)
+}
diff --git a/internal/atlas/image.go b/internal/atlas/image.go
index 8d94f18e1..5bccc45bd 100644
--- a/internal/atlas/image.go
+++ b/internal/atlas/image.go
@@ -715,6 +715,15 @@ func (i *Image) canBePutOnAtlas() bool {
 	return i.width+i.paddingSize() <= maxSize && i.height+i.paddingSize() <= maxSize
 }
 
+func (i *Image) finalize() {
+	// A function from finalizer must not be blocked, but disposing operation can be blocked.
+	// Defer this operation until it becomes safe. (#913)
+	appendDeferred(func() {
+		i.deallocate()
+		runtime.SetFinalizer(i, nil)
+	})
+}
+
 func (i *Image) allocate(forbiddenBackends []*backend, asSource bool) {
 	if !graphicsDriverInitialized {
 		panic("atlas: graphics driver must be ready at allocate but not")
@@ -724,14 +733,7 @@ func (i *Image) allocate(forbiddenBackends []*backend, asSource bool) {
 		panic("atlas: the image is already allocated")
 	}
 
-	runtime.SetFinalizer(i, func(image *Image) {
-		// A function from finalizer must not be blocked, but disposing operation can be blocked.
-		// Defer this operation until it becomes safe. (#913)
-		appendDeferred(func() {
-			image.deallocate()
-			runtime.SetFinalizer(i, nil)
-		})
-	})
+	runtime.SetFinalizer(i, (*Image).finalize)
 
 	if i.imageType == ImageTypeScreen {
 		if asSource {
diff --git a/internal/atlas/image_test.go b/internal/atlas/image_test.go
index 9f77135da..ef38db386 100644
--- a/internal/atlas/image_test.go
+++ b/internal/atlas/image_test.go
@@ -810,4 +810,17 @@ func TestIteratingImagesToPutOnSourceBackend(t *testing.T) {
 	}
 }
 
+func TestGC(t *testing.T) {
+	c0 := atlas.DeferredFuncCountForTesting()
+	img := atlas.NewImage(16, 16, atlas.ImageTypeRegular)
+	img.WritePixels(make([]byte, 4*16*16), image.Rect(0, 0, 16, 16))
+	_ = img
+	runtime.GC()
+
+	c1 := atlas.DeferredFuncCountForTesting()
+	if got, want := c1, c0+1; got != want {
+		t.Errorf("got: %d, want: %d", got, want)
+	}
+}
+
 // TODO: Add tests to extend image on an atlas out of the main loop