[BUG] SKCanvas owned by SKSurface gets disposed

[mclose90]

Description

The SKCanvas object that is originally owned by the SKSurface ends up being disposed at some point in a highly multi-threaded environment. We have seen this happen with both surfaces provided by a SKElement as well as surfaces created off screen. The issue may appear immediately or after a couple mins of running the demo. The more threads the more likely it is to happen. The debugger shows that the SKSurface still holds a canvas object but looking through source I suspect this is a newly created canvas different from the original one.

Code

<Window x:Class="WpfApp1.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:WpfApp1"
        xmlns:skia ="clr-namespace:SkiaSharp.Views.WPF;assembly=SkiaSharp.Views.WPF"
        mc:Ignorable="d"
        Title="MainWindow" Height="450" Width="800">
    <Grid>
        <skia:SKElement
            Name="Canvas"
            PaintSurface="SKElement_PaintSurface" />
    </Grid>
</Window>

using SkiaSharp;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;

namespace WpfApp1
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        const int NUM_THREADS = 16;
        SKImage _image = null;
        object _imageLock = new object();
        Random random = new Random();



        public MainWindow()
        {
            InitializeComponent();
            
            for (int i = 0; i < NUM_THREADS; i++)
                Task.Run(() =>
                {
                    for (; ; )
                        UpdateImage();
                });
        }

        private void UpdateImage()
        {
            using (var surface = SKSurface.Create(new SKImageInfo((int)ActualWidth, (int)ActualHeight, SKImageInfo.PlatformColorType, SKAlphaType.Premul)))
            {
                if (surface != null)
                {
                    var canvas = surface.Canvas;

                    canvas.Clear(SKColors.Orange);
                    using (var paint = new SKPaint() { TextSize = 136, TextAlign = SKTextAlign.Center })
                        canvas.DrawText($"{random.Next()}", new SKPoint((float)ActualWidth / 2, (float)ActualHeight / 2), paint);
                    canvas.RestoreToCount(-1);

                    var image = surface.Snapshot();

                    lock (_imageLock)
                        _image = image;
                }
            }
            Application.Current?.Dispatcher?.Invoke(() => Canvas?.InvalidateVisual());
        }

        private void SKElement_PaintSurface(object sender, SkiaSharp.Views.Desktop.SKPaintSurfaceEventArgs e)
        {
            lock (_imageLock)
            {
                if (_image != null)
                    e.Surface.Canvas.DrawImage(_image, new SKRect(0, 0, _image.Width, _image.Height), new SKRect(0, 0, e.Info.Width, e.Info.Height));
            }
        }
    }
}

Expected Behavior

The canvas should live until the surface is disposed.

Actual Behavior

The canvas is being disposed mid render sequence.

Basic Information

• Version with issue: 1.68.3 , 2.80.0 preview 14
• Last known good version: N/A
• IDE: VS 2019 Pro
• Platform Target Frameworks: .Net Standard 2.0 , .Net Framework 4.7.2, WPF, Windows 10 update 1809

Screenshots

Reproduction Link
See source code above. That encompasses the entire program to force this to happen. The more threads the more it becomes visible. May take a few seconds to a few mins of the code running to recreate.

[mattleibow]

Thanks for reporting this. I think it might be because for some reason I decided it would be better to dispose the managed objects after I told the native side to clean up.

This meant that when the new surface is created in the exact same memory location, it's canvas had the same handle. Then the var canvas = surface.Canvas; retrieved the old managed object, which was immediately disposed.

Effectively, I was a bit dumb...

I'll get a fix up ASAP and then you can let me know if it fixes things and I'll make sure it gets into the 2.80 series before it gets stable.

[mclose90]

Built the latest master and tested with our test app. Did not run into any crashes over an extended period of time. Awesome Job. Thank you!

[mattleibow]

That is good news! Thanks for the test and feedback!