shareable: Refactoring: only finalizers have to be cared

Only finalizers are problematic since they can stop everything,
but other things are not. Let's care finalizers and just use
regular locks.

Updates #913

internal/restorable/image.go

@@ -121,7 +121,8 @@ func init() {
 //
 // Note that Dispose is not called automatically.
 func NewImage(width, height int) *Image {
-	// As this should not affect the information for restoring, this doesn't have to be deferred.
+	theImages.m.Lock()
+	defer theImages.m.Unlock()
 
 	i := &Image{
 		image: graphicscommand.NewImage(width, height),
@@ -174,7 +175,8 @@ func (i *Image) MakeVolatile() {
 //
 // Note that Dispose is not called automatically.
 func NewScreenFramebufferImage(width, height int) *Image {
-	// As this should not affect the information for restoring, this doesn't have to be deferred.
+	theImages.m.Lock()
+	defer theImages.m.Unlock()
 
 	i := &Image{
 		image:  graphicscommand.NewScreenFramebufferImage(width, height),
@@ -186,18 +188,8 @@ func NewScreenFramebufferImage(width, height int) *Image {
 }
 
 func (i *Image) Clear() {
-	select {
+	theImages.m.Lock()
-	case theImages.deferCh <- struct{}{}:
+	defer theImages.m.Unlock()
-		break
-	default:
-		theImages.deferUntilBeginFrame(i.Clear)
-		return
-	}
-
-	defer func() {
-		<-theImages.deferCh
-	}()
-
 	theImages.makeStaleIfDependingOn(i)
 	i.clear()
 }
@@ -308,19 +300,8 @@ func (i *Image) ClearPixels(x, y, width, height int) {
 //
 // ReplacePixels for a part is forbidden if the image is rendered with DrawTriangles or Fill.
 func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
-	select {
+	theImages.m.Lock()
-	case theImages.deferCh <- struct{}{}:
+	defer theImages.m.Unlock()
-		break
-	default:
-		theImages.deferUntilBeginFrame(func() {
-			i.ReplacePixels(pixels, x, y, width, height)
-		})
-		return
-	}
-
-	defer func() {
-		<-theImages.deferCh
-	}()
 
 	w, h := i.image.Size()
 	if width <= 0 || height <= 0 {
@@ -375,19 +356,8 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 
 // DrawTriangles draws a given image img to the image.
 func (i *Image) DrawTriangles(img *Image, vertices []float32, indices []uint16, colorm *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address) {
-	select {
+	theImages.m.Lock()
-	case theImages.deferCh <- struct{}{}:
+	defer theImages.m.Unlock()
-		break
-	default:
-		theImages.deferUntilBeginFrame(func() {
-			i.DrawTriangles(img, vertices, indices, colorm, mode, filter, address)
-		})
-		return
-	}
-
-	defer func() {
-		<-theImages.deferCh
-	}()
 
 	if i.priority {
 		panic("restorable: DrawTriangles cannot be called on a priority image")
@@ -443,11 +413,8 @@ func (i *Image) readPixelsFromGPUIfNeeded() {
 //
 // Note that this must not be called until context is available.
 func (i *Image) At(x, y int) (byte, byte, byte, byte) {
-	// As At can be affected by deferred functions, and At itself cannot be deferred, lock this operation.
+	theImages.m.Lock()
-	theImages.deferCh <- struct{}{}
+	defer theImages.m.Unlock()
-	defer func() {
-		<-theImages.deferCh
-	}()
 
 	w, h := i.image.Size()
 	if x < 0 || y < 0 || w <= x || h <= y {
@@ -574,16 +541,8 @@ func (i *Image) restore() {
 //
 // After disposing, calling the function of the image causes unexpected results.
 func (i *Image) Dispose() {
-	select {
+	theImages.m.Lock()
-	case theImages.deferCh <- struct{}{}:
+	defer theImages.m.Unlock()
-		break
-	default:
-		theImages.deferUntilBeginFrame(i.Dispose)
-		return
-	}
-	defer func() {
-		<-theImages.deferCh
-	}()
 
 	theImages.remove(i)
 	i.image.Dispose()

internal/restorable/images.go

@@ -42,17 +42,13 @@ type images struct {
 	images     map[*Image]struct{}
 	lastTarget *Image
 
-	deferred  []func()
+	m    sync.Mutex
-	mDeferred sync.Mutex
-	deferCh   chan struct{}
-
 	once sync.Once
 }
 
 // theImages represents the images for the current process.
 var theImages = &images{
-	images:  map[*Image]struct{}{},
+	images: map[*Image]struct{}{},
-	deferCh: make(chan struct{}, 1),
 }
 
 // ResolveStaleImages flushes the queued draw commands and resolves
@@ -62,7 +58,7 @@ var theImages = &images{
 func ResolveStaleImages() {
 	defer func() {
 		// Until the begin o the frame (by RestoreIfNeeded, any operations are deferred.
-		theImages.deferCh <- struct{}{}
+		theImages.m.Lock()
 	}()
 
 	graphicscommand.FlushCommands()
@@ -89,12 +85,9 @@ func RestoreIfNeeded() error {
 		firsttime = true
 	})
 	if !firsttime {
-		<-theImages.deferCh
+		theImages.m.Unlock()
 	}
 
-	// Deferred functions should be resolved after restoring.
-	defer theImages.resolveDeferred()
-
 	if !needsRestoring() {
 		return nil
 	}
@@ -146,22 +139,6 @@ func (i *images) remove(img *Image) {
 	delete(i.images, img)
 }
 
-func (i *images) deferUntilBeginFrame(f func()) {
-	i.mDeferred.Lock()
-	defer i.mDeferred.Unlock()
-	i.deferred = append(i.deferred, f)
-}
-
-func (i *images) resolveDeferred() {
-	i.mDeferred.Lock()
-	defer i.mDeferred.Unlock()
-
-	for _, f := range i.deferred {
-		f()
-	}
-	i.deferred = nil
-}
-
 // resolveStaleImages resolves stale images.
 func (i *images) resolveStaleImages() {
 	i.lastTarget = nil

internal/shareable/shareable.go

@@ -382,20 +382,27 @@ func (i *Image) at(x, y int) (byte, byte, byte, byte) {
 	return i.backend.restorable.At(x+ox, y+oy)
 }
 
-func (i *Image) Dispose() {
+// disposeFromFinalizer disposes images, but the actual operation is deferred.
+// disposeFromFinalizer is called from finalizers.
+//
+// A function from finalizer must not be blocked, but disposing operation can be blocked.
+// Defer this operation until it becomes safe. (#913)
+func (i *Image) disposeFromFinalizer() {
 	deferredM.Lock()
 	defer deferredM.Unlock()
 
-	// Defer actual disposing until disposing can finish completely.
-	// Otherwise, ClearPixels can be deferred in between frames, and delayed ClearPixels can affect other images.
-	// (#913)
-	//
-	// TODO: Other operations should be deferred too?
 	deferred = append(deferred, func() {
 		i.dispose(true)
 	})
 }
 
+func (i *Image) Dispose() {
+	backendsM.Lock()
+	defer backendsM.Unlock()
+
+	i.dispose(true)
+}
+
 func (i *Image) dispose(markDisposed bool) {
 	defer func() {
 		if markDisposed {
@@ -480,7 +487,7 @@ func (i *Image) allocate(shareable bool) {
 		i.backend = &backend{
 			restorable: restorable.NewImage(i.width, i.height),
 		}
-		runtime.SetFinalizer(i, (*Image).Dispose)
+		runtime.SetFinalizer(i, (*Image).disposeFromFinalizer)
 		return
 	}
 
@@ -488,7 +495,7 @@ func (i *Image) allocate(shareable bool) {
 		if n, ok := b.TryAlloc(i.width, i.height); ok {
 			i.backend = b
 			i.node = n
-			runtime.SetFinalizer(i, (*Image).Dispose)
+			runtime.SetFinalizer(i, (*Image).disposeFromFinalizer)
 			return
 		}
 	}
@@ -512,7 +519,7 @@ func (i *Image) allocate(shareable bool) {
 	}
 	i.backend = b
 	i.node = n
-	runtime.SetFinalizer(i, (*Image).Dispose)
+	runtime.SetFinalizer(i, (*Image).disposeFromFinalizer)
 }
 
 func (i *Image) MakeVolatile() {
@@ -544,7 +551,7 @@ func NewScreenFramebufferImage(width, height int) *Image {
 		},
 		neverShared: true,
 	}
-	runtime.SetFinalizer(i, (*Image).Dispose)
+	runtime.SetFinalizer(i, (*Image).disposeFromFinalizer)
 	return i
 }