From ade7b8ceec95746064dbb058b46fe1f8b4cf2c64 Mon Sep 17 00:00:00 2001
From: Hajime Hoshi <hajimehoshi@gmail.com>
Date: Tue, 21 Feb 2023 15:51:46 +0900
Subject: [PATCH] internal/buffered: bug fix: functions were not
 concurrent-safe

Before this change, `delayedCommandsFlushed` was set as 1 BEFORE
buffered commands were flushed. This meant that a function call and
a command being flushed were not concurrent-safe.

This change fixes this issue by changing the timing of setting
`delayedCommandsFlushed` as 1 to the later time after flushing the
buffered commands.

Closes #2580
---
 internal/buffered/command.go | 28 +++++++---------------------
 internal/buffered/image.go   | 34 ++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/internal/buffered/command.go b/internal/buffered/command.go
index afe9b4ea9..5d0c708b5 100644
--- a/internal/buffered/command.go
+++ b/internal/buffered/command.go
@@ -30,34 +30,22 @@ var (
 )
 
 func flushDelayedCommands() {
-	fs := getDelayedFuncsAndClear()
-	for _, f := range fs {
-		f()
-	}
-}
-
-func getDelayedFuncsAndClear() []func() {
 	if atomic.LoadUint32(&delayedCommandsFlushed) == 0 {
 		// Outline the slow-path to expect the fast-path is inlined.
-		return getDelayedFuncsAndClearSlow()
+		flushDelayedCommandsSlow()
 	}
-	return nil
 }
 
-func getDelayedFuncsAndClearSlow() []func() {
+func flushDelayedCommandsSlow() {
 	delayedCommandsM.Lock()
 	defer delayedCommandsM.Unlock()
 
 	if delayedCommandsFlushed == 0 {
-		defer atomic.StoreUint32(&delayedCommandsFlushed, 1)
-
-		fs := make([]func(), len(delayedCommands))
-		copy(fs, delayedCommands)
-		delayedCommands = nil
-		return fs
+		for _, f := range delayedCommands {
+			f()
+		}
+		delayedCommandsFlushed = 1
 	}
-
-	return nil
 }
 
 // maybeCanAddDelayedCommand returns false if the delayed commands cannot be added.
@@ -72,9 +60,7 @@ func tryAddDelayedCommand(f func()) bool {
 	defer delayedCommandsM.Unlock()
 
 	if delayedCommandsFlushed == 0 {
-		delayedCommands = append(delayedCommands, func() {
-			f()
-		})
+		delayedCommands = append(delayedCommands, f)
 		return true
 	}
 
diff --git a/internal/buffered/image.go b/internal/buffered/image.go
index 0fbcb82c4..2c0547edd 100644
--- a/internal/buffered/image.go
+++ b/internal/buffered/image.go
@@ -57,11 +57,15 @@ func NewImage(width, height int, imageType atlas.ImageType) *Image {
 func (i *Image) initialize(imageType atlas.ImageType) {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			i.initialize(imageType)
+			i.initializeImpl(imageType)
 		}) {
 			return
 		}
 	}
+	i.initializeImpl(imageType)
+}
+
+func (i *Image) initializeImpl(imageType atlas.ImageType) {
 	i.img = atlas.NewImage(i.width, i.height, imageType)
 }
 
@@ -72,11 +76,15 @@ func (i *Image) invalidatePixels() {
 func (i *Image) MarkDisposed() {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			i.MarkDisposed()
+			i.markDisposedImpl()
 		}) {
 			return
 		}
 	}
+	i.markDisposedImpl()
+}
+
+func (i *Image) markDisposedImpl() {
 	i.invalidatePixels()
 	i.img.MarkDisposed()
 }
@@ -128,12 +136,15 @@ func (i *Image) WritePixels(pix []byte, x, y, width, height int) {
 		copied := make([]byte, len(pix))
 		copy(copied, pix)
 		if tryAddDelayedCommand(func() {
-			i.WritePixels(copied, x, y, width, height)
+			i.writePixelsImpl(copied, x, y, width, height)
 		}) {
 			return
 		}
 	}
+	i.writePixelsImpl(pix, x, y, width, height)
+}
 
+func (i *Image) writePixelsImpl(pix []byte, x, y, width, height int) {
 	i.invalidatePixels()
 	i.img.WritePixels(pix, x, y, width, height)
 }
@@ -151,12 +162,15 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageCount]*Image, vertices [
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
 			// Arguments are not copied. Copying is the caller's responsibility.
-			i.DrawTriangles(srcs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, shader, uniforms, evenOdd)
+			i.drawTrianglesImpl(srcs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, shader, uniforms, evenOdd)
 		}) {
 			return
 		}
 	}
+	i.drawTrianglesImpl(srcs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, shader, uniforms, evenOdd)
+}
 
+func (i *Image) drawTrianglesImpl(srcs [graphics.ShaderImageCount]*Image, vertices []float32, indices []uint16, colorm affine.ColorM, mode graphicsdriver.CompositeMode, filter graphicsdriver.Filter, address graphicsdriver.Address, dstRegion, srcRegion graphicsdriver.Region, subimageOffsets [graphics.ShaderImageCount - 1][2]float32, shader *Shader, uniforms [][]float32, evenOdd bool) {
 	var s *atlas.Shader
 	var imgs [graphics.ShaderImageCount]*atlas.Image
 	if shader == nil {
@@ -190,22 +204,30 @@ func NewShader(ir *shaderir.Program) *Shader {
 func (s *Shader) initialize(ir *shaderir.Program) {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			s.initialize(ir)
+			s.initializeImpl(ir)
 		}) {
 			return
 		}
 	}
+	s.initializeImpl(ir)
+}
+
+func (s *Shader) initializeImpl(ir *shaderir.Program) {
 	s.shader = atlas.NewShader(ir)
 }
 
 func (s *Shader) MarkDisposed() {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			s.MarkDisposed()
+			s.markDisposedImpl()
 		}) {
 			return
 		}
 	}
+	s.markDisposedImpl()
+}
+
+func (s *Shader) markDisposedImpl() {
 	s.shader.MarkDisposed()
 	s.shader = nil
 }