shareable: Bug fix: Crash when syncing pixels of a disposed image

When an image was disposed, the image was not unregistered from the
set of 'imagesToMakeShared'. This caused a crash when trying to sync
the pixel data of the disposed image.

This change fixes the issue by unregistering the image when the
image is disposed.

Fixes #1421
This commit is contained in:
Hajime Hoshi 2020-11-12 23:37:49 +09:00
parent ddfb8adbc4
commit 0366103b2e
3 changed files with 60 additions and 1 deletions

View File

@ -52,3 +52,7 @@ func (i *Image) EnsureNotSharedForTesting() {
defer backendsM.Unlock() defer backendsM.Unlock()
i.ensureNotShared() i.ensureNotShared()
} }
func ResolveDeferredForTesting() {
resolveDeferred()
}

View File

@ -71,10 +71,15 @@ func resolveDeferred() {
// This value is exported for testing. // This value is exported for testing.
const MaxCountForShare = 10 const MaxCountForShare = 10
// CountForStartSyncing represents the count that the image starts to sync pixels between GPU and CPU.
//
// This value is exported for testing.
const CountForStartSyncing = MaxCountForShare / 2
func makeImagesShared() error { func makeImagesShared() error {
for i := range imagesToMakeShared { for i := range imagesToMakeShared {
i.nonUpdatedCount++ i.nonUpdatedCount++
if i.nonUpdatedCount >= MaxCountForShare/2 && i.syncing == nil { if i.nonUpdatedCount >= CountForStartSyncing && i.syncing == nil {
// Sync the pixel data on CPU and GPU sides explicitly in order not to block this process. // Sync the pixel data on CPU and GPU sides explicitly in order not to block this process.
ch, err := i.backend.restorable.Sync() ch, err := i.backend.restorable.Sync()
if err != nil { if err != nil {
@ -523,6 +528,8 @@ func (i *Image) dispose(markDisposed bool) {
} }
}() }()
i.resetNonUpdatedCount()
if i.disposed { if i.disposed {
return return
} }

View File

@ -538,4 +538,52 @@ func TestMinImageSize(t *testing.T) {
img.ReplacePixels(make([]byte, 4*s*s)) img.ReplacePixels(make([]byte, 4*s*s))
} }
// Issue #1421
func TestDisposedAndReshared(t *testing.T) {
const size = 16
src := NewImage(size, size)
defer src.MarkDisposed()
src2 := NewImage(size, size)
defer src2.MarkDisposed()
dst := NewImage(size, size)
defer dst.MarkDisposed()
// Use src as a render target so that src is not on the shared image.
vs := quadVertices(size, size, 0, 0, 1)
is := graphics.QuadIndices()
dr := driver.Region{
X: 0,
Y: 0,
Width: size,
Height: size,
}
src.DrawTriangles([graphics.ShaderImageNum]*Image{src2}, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, [graphics.ShaderImageNum - 1][2]float32{}, nil, nil)
if got, want := src.IsSharedForTesting(), false; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
// Use src as a render source.
for i := 0; i < CountForStartSyncing; i++ {
if err := MakeImagesSharedForTesting(); err != nil {
t.Fatal(err)
}
dst.DrawTriangles([graphics.ShaderImageNum]*Image{src}, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, [graphics.ShaderImageNum - 1][2]float32{}, nil, nil)
if got, want := src.IsSharedForTesting(), false; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
}
// Before MakeImagesSharedForTesting, dispose the image.
src.MarkDisposed()
// Force to dispose the image.
ResolveDeferredForTesting()
// Confirm that MakeImagesSharedForTesting doesn't panic.
if err := MakeImagesSharedForTesting(); err != nil {
t.Fatal(err)
}
}
// TODO: Add tests to extend shareable image out of the main loop // TODO: Add tests to extend shareable image out of the main loop