shareable: Rename Dispose -> MarkDisposed

Before this change, it was not clear that shareable's Dispose can
be called from finalizers since finalizers must not be blocked by
a mutex. Actually Dispose could be locked and must not be called
from finalizers.

This change renames the function to avoid confusion of the API,
and make the function available from finalizers.
This commit is contained in:
Hajime Hoshi 2019-09-21 19:54:39 +09:00
parent 053f5a0ce7
commit 9d867850dc
3 changed files with 37 additions and 35 deletions

View File

@ -58,10 +58,14 @@ func init() {
} }
func resolveDeferred() { func resolveDeferred() {
for _, f := range deferred { deferredM.Lock()
fs := deferred
deferred = nil
deferredM.Unlock()
for _, f := range fs {
f() f()
} }
deferred = nil
} }
// MaxCountForShare represents the time duration when the image can become shared. // MaxCountForShare represents the time duration when the image can become shared.
@ -132,6 +136,9 @@ var (
imagesToMakeShared = map[*Image]struct{}{} imagesToMakeShared = map[*Image]struct{}{}
deferred []func() deferred []func()
// deferredM is a mutext for the slice operations. This must not be used for other usages.
deferredM sync.Mutex
) )
func init() { func init() {
@ -395,23 +402,18 @@ func (i *Image) at(x, y int) (byte, byte, byte, byte) {
return i.backend.restorable.At(x+ox, y+oy) return i.backend.restorable.At(x+ox, y+oy)
} }
// disposeFromFinalizer disposes images, but the actual operation is deferred. // MarkDisposed marks the image as disposed. The actual operation is deferred.
// disposeFromFinalizer is called from finalizers. // MarkDisposed can be called from finalizers.
// //
// A function from finalizer must not be blocked, but disposing operation can be blocked. // A function from finalizer must not be blocked, but disposing operation can be blocked.
// Defer this operation until it becomes safe. (#913) // Defer this operation until it becomes safe. (#913)
func (i *Image) disposeFromFinalizer() { func (i *Image) MarkDisposed() {
// deferred doesn't have to be, and should not be protected by a mutex. // deferred doesn't have to be, and should not be protected by a mutex.
deferredM.Lock()
deferred = append(deferred, func() { deferred = append(deferred, func() {
i.dispose(true) i.dispose(true)
}) })
} deferredM.Unlock()
func (i *Image) Dispose() {
backendsM.Lock()
defer backendsM.Unlock()
i.dispose(true)
} }
func (i *Image) dispose(markDisposed bool) { func (i *Image) dispose(markDisposed bool) {
@ -488,7 +490,7 @@ func (i *Image) allocate(shareable bool) {
panic("shareable: the image is already allocated") panic("shareable: the image is already allocated")
} }
runtime.SetFinalizer(i, (*Image).disposeFromFinalizer) runtime.SetFinalizer(i, (*Image).MarkDisposed)
if i.screen { if i.screen {
i.backend = &backend{ i.backend = &backend{

View File

@ -67,22 +67,22 @@ func TestEnsureNotShared(t *testing.T) {
// Create img1 and img2 with this size so that the next images are allocated // Create img1 and img2 with this size so that the next images are allocated
// with non-upper-left location. // with non-upper-left location.
img1 := NewImage(bigSize, 100, false) img1 := NewImage(bigSize, 100, false)
defer img1.Dispose() defer img1.MarkDisposed()
// Ensure img1's region is allocated. // Ensure img1's region is allocated.
img1.ReplacePixels(make([]byte, 4*bigSize*100)) img1.ReplacePixels(make([]byte, 4*bigSize*100))
img2 := NewImage(100, bigSize, false) img2 := NewImage(100, bigSize, false)
defer img2.Dispose() defer img2.MarkDisposed()
img2.ReplacePixels(make([]byte, 4*100*bigSize)) img2.ReplacePixels(make([]byte, 4*100*bigSize))
const size = 32 const size = 32
img3 := NewImage(size/2, size/2, false) img3 := NewImage(size/2, size/2, false)
defer img3.Dispose() defer img3.MarkDisposed()
img3.ReplacePixels(make([]byte, (size/2)*(size/2)*4)) img3.ReplacePixels(make([]byte, (size/2)*(size/2)*4))
img4 := NewImage(size, size, false) img4 := NewImage(size, size, false)
defer img4.Dispose() defer img4.MarkDisposed()
pix := make([]byte, size*size*4) pix := make([]byte, size*size*4)
for j := 0; j < size; j++ { for j := 0; j < size; j++ {
@ -134,18 +134,18 @@ func TestReshared(t *testing.T) {
const size = 16 const size = 16
img0 := NewImage(size, size, false) img0 := NewImage(size, size, false)
defer img0.Dispose() defer img0.MarkDisposed()
img0.ReplacePixels(make([]byte, 4*size*size)) img0.ReplacePixels(make([]byte, 4*size*size))
img1 := NewImage(size, size, false) img1 := NewImage(size, size, false)
defer img1.Dispose() defer img1.MarkDisposed()
img1.ReplacePixels(make([]byte, 4*size*size)) img1.ReplacePixels(make([]byte, 4*size*size))
if got, want := img1.IsSharedForTesting(), true; got != want { if got, want := img1.IsSharedForTesting(), true; got != want {
t.Errorf("got: %v, want: %v", got, want) t.Errorf("got: %v, want: %v", got, want)
} }
img2 := NewImage(size, size, false) img2 := NewImage(size, size, false)
defer img2.Dispose() defer img2.MarkDisposed()
pix := make([]byte, 4*size*size) pix := make([]byte, 4*size*size)
for j := 0; j < size; j++ { for j := 0; j < size; j++ {
for i := 0; i < size; i++ { for i := 0; i < size; i++ {
@ -158,7 +158,7 @@ func TestReshared(t *testing.T) {
img2.ReplacePixels(pix) img2.ReplacePixels(pix)
img3 := NewImage(size, size, true /* volatile */) img3 := NewImage(size, size, true /* volatile */)
defer img3.Dispose() defer img3.MarkDisposed()
img1.ReplacePixels(make([]byte, 4*size*size)) img1.ReplacePixels(make([]byte, 4*size*size))
if got, want := img3.IsSharedForTesting(), false; got != want { if got, want := img3.IsSharedForTesting(), false; got != want {
t.Errorf("got: %v, want: %v", got, want) t.Errorf("got: %v, want: %v", got, want)
@ -224,7 +224,7 @@ func TestReshared(t *testing.T) {
func TestExtend(t *testing.T) { func TestExtend(t *testing.T) {
const w0, h0 = 100, 100 const w0, h0 = 100, 100
img0 := NewImage(w0, h0, false) img0 := NewImage(w0, h0, false)
defer img0.Dispose() defer img0.MarkDisposed()
p0 := make([]byte, 4*w0*h0) p0 := make([]byte, 4*w0*h0)
for i := 0; i < w0*h0; i++ { for i := 0; i < w0*h0; i++ {
p0[4*i] = byte(i) p0[4*i] = byte(i)
@ -236,7 +236,7 @@ func TestExtend(t *testing.T) {
const w1, h1 = 1025, 100 const w1, h1 = 1025, 100
img1 := NewImage(w1, h1, false) img1 := NewImage(w1, h1, false)
defer img1.Dispose() defer img1.MarkDisposed()
p1 := make([]byte, 4*w1*h1) p1 := make([]byte, 4*w1*h1)
for i := 0; i < w1*h1; i++ { for i := 0; i < w1*h1; i++ {
p1[4*i] = byte(i) p1[4*i] = byte(i)
@ -271,16 +271,16 @@ func TestExtend(t *testing.T) {
} }
} }
img0.Dispose() img0.MarkDisposed()
img1.Dispose() img1.MarkDisposed()
} }
func TestReplacePixelsAfterDrawTriangles(t *testing.T) { func TestReplacePixelsAfterDrawTriangles(t *testing.T) {
const w, h = 256, 256 const w, h = 256, 256
src := NewImage(w, h, false) src := NewImage(w, h, false)
defer src.Dispose() defer src.MarkDisposed()
dst := NewImage(w, h, false) dst := NewImage(w, h, false)
defer dst.Dispose() defer dst.MarkDisposed()
pix := make([]byte, 4*w*h) pix := make([]byte, 4*w*h)
for i := 0; i < w*h; i++ { for i := 0; i < w*h; i++ {
@ -313,9 +313,9 @@ func TestReplacePixelsAfterDrawTriangles(t *testing.T) {
func TestSmallImages(t *testing.T) { func TestSmallImages(t *testing.T) {
const w, h = 4, 8 const w, h = 4, 8
src := NewImage(w, h, false) src := NewImage(w, h, false)
defer src.Dispose() defer src.MarkDisposed()
dst := NewImage(w, h, false) dst := NewImage(w, h, false)
defer dst.Dispose() defer dst.MarkDisposed()
pix := make([]byte, 4*w*h) pix := make([]byte, 4*w*h)
for i := 0; i < w*h; i++ { for i := 0; i < w*h; i++ {
@ -347,9 +347,9 @@ func TestSmallImages(t *testing.T) {
func TestLongImages(t *testing.T) { func TestLongImages(t *testing.T) {
const w, h = 1, 6 const w, h = 1, 6
src := NewImage(w, h, false) src := NewImage(w, h, false)
defer src.Dispose() defer src.MarkDisposed()
dst := NewImage(256, 256, false) dst := NewImage(256, 256, false)
defer dst.Dispose() defer dst.MarkDisposed()
pix := make([]byte, 4*w*h) pix := make([]byte, 4*w*h)
for i := 0; i < w*h; i++ { for i := 0; i < w*h; i++ {
@ -389,8 +389,8 @@ func TestDisposeImmediately(t *testing.T) {
// img0 and img1 should share the same backend in 99.9999% possibility. // img0 and img1 should share the same backend in 99.9999% possibility.
img0.Dispose() img0.MarkDisposed()
img1.Dispose() img1.MarkDisposed()
} }
// TODO: Add tests to extend shareable image out of the main loop // TODO: Add tests to extend shareable image out of the main loop

View File

@ -259,14 +259,14 @@ func (m *mipmap) isDisposed() bool {
func (m *mipmap) dispose() { func (m *mipmap) dispose() {
m.disposeMipmaps() m.disposeMipmaps()
m.orig.Dispose() m.orig.MarkDisposed()
m.orig = nil m.orig = nil
} }
func (m *mipmap) disposeMipmaps() { func (m *mipmap) disposeMipmaps() {
for _, a := range m.imgs { for _, a := range m.imgs {
for _, img := range a { for _, img := range a {
img.Dispose() img.MarkDisposed()
} }
} }
for k := range m.imgs { for k := range m.imgs {