buffered: Bug fix: Race conndition on delayedCommands

This CL fixes the race condtion on delayedCommands, which can be
accessed and set to nil at the same time.

This CL separates some operations for delayedCommands into slow-
paths and fast-paths, and use mutex only at slow-paths for
performance. The implementation is based on sync.Once.

Fixes #1195
This commit is contained in:
Hajime Hoshi 2020-06-16 22:44:47 +09:00
parent 1735dda586
commit d9cf1095d4
2 changed files with 92 additions and 58 deletions

View File

@ -15,16 +15,18 @@
package buffered package buffered
import ( import (
"sync"
"sync/atomic" "sync/atomic"
) )
var ( var (
needsToDelayCommandsV int32 = 1
// delayedCommands represents a queue for image operations that are ordered before the game starts // delayedCommands represents a queue for image operations that are ordered before the game starts
// (BeginFrame). Before the game starts, the package shareable doesn't determine the minimum/maximum texture // (BeginFrame). Before the game starts, the package shareable doesn't determine the minimum/maximum texture
// sizes (#879). // sizes (#879).
delayedCommands []func() error delayedCommands = []func() error{}
delayedCommandsM sync.Mutex
delayedCommandsFlushed uint32
) )
func flushDelayedCommands() error { func flushDelayedCommands() error {
@ -39,7 +41,19 @@ func flushDelayedCommands() error {
} }
func getDelayedFuncsAndClear() []func() error { func getDelayedFuncsAndClear() []func() error {
atomic.StoreInt32(&needsToDelayCommandsV, 0) if atomic.LoadUint32(&delayedCommandsFlushed) == 0 {
// Outline the slow-path to expect the fast-path is inlined.
return getDelayedFuncsAndClearSlow()
}
return nil
}
func getDelayedFuncsAndClearSlow() []func() error {
delayedCommandsM.Lock()
defer delayedCommandsM.Unlock()
if delayedCommandsFlushed == 0 {
defer atomic.StoreUint32(&delayedCommandsFlushed, 1)
fs := make([]func() error, len(delayedCommands)) fs := make([]func() error, len(delayedCommands))
copy(fs, delayedCommands) copy(fs, delayedCommands)
@ -47,6 +61,36 @@ func getDelayedFuncsAndClear() []func() error {
return fs return fs
} }
func needsToDelayCommands() bool { return nil
return atomic.LoadInt32(&needsToDelayCommandsV) != 0 }
func tryAddDelayedCommand(f func(obj interface{}) error, ondelayed func() interface{}) bool {
if atomic.LoadUint32(&delayedCommandsFlushed) == 0 {
// Outline the slow-path to expect the fast-path is inlined.
tryAddDelayedCommandSlow(f, ondelayed)
return true
}
return false
}
func tryAddDelayedCommandSlow(f func(obj interface{}) error, ondelayed func() interface{}) {
delayedCommandsM.Lock()
defer delayedCommandsM.Unlock()
if delayedCommandsFlushed == 0 {
var obj interface{}
if ondelayed != nil {
obj = ondelayed()
}
delayedCommands = append(delayedCommands, func() error {
return f(obj)
})
}
}
func checkDelayedCommandsNil(fname string) {
if delayedCommands != nil {
panic("buffered: the command queue is not available yet at " + fname)
}
} }

View File

@ -55,11 +55,10 @@ func NewImage(width, height int, volatile bool) *Image {
} }
func (i *Image) initialize(width, height int, volatile bool) { func (i *Image) initialize(width, height int, volatile bool) {
if needsToDelayCommands() { if tryAddDelayedCommand(func(obj interface{}) error {
delayedCommands = append(delayedCommands, func() error {
i.initialize(width, height, volatile) i.initialize(width, height, volatile)
return nil return nil
}) }, nil) {
return return
} }
i.img = mipmap.New(width, height, volatile) i.img = mipmap.New(width, height, volatile)
@ -74,11 +73,10 @@ func NewScreenFramebufferImage(width, height int) *Image {
} }
func (i *Image) initializeAsScreenFramebuffer(width, height int) { func (i *Image) initializeAsScreenFramebuffer(width, height int) {
if needsToDelayCommands() { if tryAddDelayedCommand(func(obj interface{}) error {
delayedCommands = append(delayedCommands, func() error {
i.initializeAsScreenFramebuffer(width, height) i.initializeAsScreenFramebuffer(width, height)
return nil return nil
}) }, nil) {
return return
} }
@ -116,11 +114,10 @@ func (i *Image) resolvePendingFill() {
} }
func (i *Image) MarkDisposed() { func (i *Image) MarkDisposed() {
if needsToDelayCommands() { if tryAddDelayedCommand(func(obj interface{}) error {
delayedCommands = append(delayedCommands, func() error {
i.MarkDisposed() i.MarkDisposed()
return nil return nil
}) }, nil) {
return return
} }
i.invalidatePendingPixels() i.invalidatePendingPixels()
@ -128,9 +125,7 @@ func (i *Image) MarkDisposed() {
} }
func (img *Image) Pixels(x, y, width, height int) (pix []byte, err error) { func (img *Image) Pixels(x, y, width, height int) (pix []byte, err error) {
if needsToDelayCommands() { checkDelayedCommandsNil("Pixels")
panic("buffered: the command queue is not available yet at At")
}
if !image.Rect(x, y, x+width, y+height).In(image.Rect(0, 0, img.width, img.height)) { if !image.Rect(x, y, x+width, y+height).In(image.Rect(0, 0, img.width, img.height)) {
return nil, fmt.Errorf("buffered: out of range") return nil, fmt.Errorf("buffered: out of range")
@ -165,18 +160,15 @@ func (img *Image) Pixels(x, y, width, height int) (pix []byte, err error) {
} }
func (i *Image) Dump(name string, blackbg bool) error { func (i *Image) Dump(name string, blackbg bool) error {
if needsToDelayCommands() { checkDelayedCommandsNil("Dump")
panic("buffered: the command queue is not available yet at Dump")
}
return i.img.Dump(name, blackbg) return i.img.Dump(name, blackbg)
} }
func (i *Image) Fill(clr color.RGBA) { func (i *Image) Fill(clr color.RGBA) {
if needsToDelayCommands() { if tryAddDelayedCommand(func(obj interface{}) error {
delayedCommands = append(delayedCommands, func() error {
i.Fill(clr) i.Fill(clr)
return nil return nil
}) }, nil) {
return return
} }
@ -191,13 +183,14 @@ func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) error {
panic(fmt.Sprintf("buffered: len(pix) was %d but must be %d", len(pix), l)) panic(fmt.Sprintf("buffered: len(pix) was %d but must be %d", len(pix), l))
} }
if needsToDelayCommands() { if tryAddDelayedCommand(func(copied interface{}) error {
i.ReplacePixels(copied.([]byte), x, y, width, height)
return nil
}, func() interface{} {
copied := make([]byte, len(pix)) copied := make([]byte, len(pix))
copy(copied, pix) copy(copied, pix)
delayedCommands = append(delayedCommands, func() error { return copied
i.ReplacePixels(copied, x, y, width, height) }) {
return nil
})
return nil return nil
} }
@ -237,11 +230,10 @@ func (i *Image) replacePendingPixels(pix []byte, x, y, width, height int) {
} }
func (i *Image) CopyPixels(img *Image, x, y, width, height int) error { func (i *Image) CopyPixels(img *Image, x, y, width, height int) error {
if needsToDelayCommands() { if tryAddDelayedCommand(func(obj interface{}) error {
delayedCommands = append(delayedCommands, func() error {
i.CopyPixels(img, x, y, width, height) i.CopyPixels(img, x, y, width, height)
return nil return nil
}) }, nil) {
return nil return nil
} }
@ -269,11 +261,10 @@ func (i *Image) DrawImage(src *Image, bounds image.Rectangle, a, b, c, d, tx, ty
Ty: ty, Ty: ty,
} }
if needsToDelayCommands() { if tryAddDelayedCommand(func(obj interface{}) error {
delayedCommands = append(delayedCommands, func() error {
i.drawImage(src, bounds, g, colorm, mode, filter) i.drawImage(src, bounds, g, colorm, mode, filter)
return nil return nil
}) }, nil) {
return return
} }
@ -306,12 +297,11 @@ func (i *Image) DrawTriangles(src *Image, vertices []float32, indices []uint16,
} }
} }
if needsToDelayCommands() { if tryAddDelayedCommand(func(obj interface{}) error {
delayedCommands = append(delayedCommands, func() error {
// Arguments are not copied. Copying is the caller's responsibility. // Arguments are not copied. Copying is the caller's responsibility.
i.DrawTriangles(src, vertices, indices, colorm, mode, filter, address, shader, uniforms) i.DrawTriangles(src, vertices, indices, colorm, mode, filter, address, shader, uniforms)
return nil return nil
}) }, nil) {
return return
} }