[BUG] AccessViolationException in UWP when navigating to a page with SKGLView

[kerego]

Description
UWP app that has a page done in Xamarin.Forms using SkiaSharp.SKGLView
There are two pages, MainPage(native uwp) and XappWrapper(xamarin.forms)
DrawInBackground is set to true
Navigate forward and back a few times.

Actual Behavior
App crashes with the following Exception:
System.AccessViolationException: 'Attempted to read or write protected memory. This is often an indication that other memory is corrupt.'

Expected Behavior
No exception is thrown
Setting DrawInBackground = false, fixes the issue, but severely affects performance.

Basic Information

• Version with issue: 1.68.3 - 2.80.1
• Last known good version: 1.68.0
• IDE: Visual Studio
• Platform Target Frameworks:
  • UWP: 16299
• Target Devices: Windows PC

Detailed IDE/OS information (click to expand)

Microsoft Windows 2004 (OS Build 19041.450)
Microsoft Visual Studio Community 2019 Version 16.6.1
Xamarin 16.6.000.1061 (d16-6@8e68aa8)

Reproduction Project

MemoryViolationRepro.zip

[mattleibow]

Thanks for this issue, I'll have a look and see what is up.

This sounds like the thread is trying to draw on a disposed surface or something.

[kerego]

I've investigated this a bit and it seems that the issue was first introduced with 8039d0d commit.
For some reason, after destroying the eglContext it tends to fail on the next call to MakeCurrent().
Commenting out the GlesContext.Cleanup() does fix the crash issue but probably introduces other issues (mem leak).
Hope this helps in your investigation. I am happy to help if you need any additional info.

[MartinZikmund]

Hey, we have this problem as well in our Physics apps, and I have a simple repro in blank UWP app:

MainPage.xaml

<Grid>
    <Button VerticalAlignment="Top" Click="GoClick">Go</Button>
</Grid>

MainPage.xaml.cs

private void GoClick(object sender, RoutedEventArgs e)
{
    Frame.Navigate(typeof(SecondPage));
}

SecondPage.xaml

<Grid>
    <Grid.RowDefinitions>
        <RowDefinition Height="Auto" />
        <RowDefinition Height="*" />
    </Grid.RowDefinitions>
    <Button Click="Button_Click">Back</Button>
    <Grid x:Name="CanvasHolder" Grid.Row="1">
        <uwp:SKSwapChainPanel DrawInBackground="True" />
    </Grid>
</Grid

After clicking Go and Back the app will crash with AccessViolationException after 2-3 attempts.

Has someone found a workaround that would allow to keep DrawInBackground=True while avoiding the error? Unfortunately we cannot downgrade to older versions due to dependencies, but the app now crashes quite consistently on second or third navigation between pages 😅

[MartinZikmund]

@mattleibow Do you have some idea how to workaround this (for example a way to swallow the exception somewhere)? I have tried try..catch around construction of the canvas, and around navigation to the page, but the exception probably occurs on a different thread.

Oh, and happy new year 🎆 !

[MartinZikmund]

@mattleibow as this is issue is now a blocker for prod release of a project I am working on, I tried to look into this issue, but I am not successful so far. Here are some observations I have made so far.

• The exception occurs when ThreadPool.RunAsync(RenderOnce) or ThreadPool.RunAsync(RenderLoop) plan the new AngleSwapChainPanel work on a thread that was already used before. So if I navigate to a page first time and the background thread is thread 2, later navigate again and the new control gets thread 3, the exception does not occur. However, if I later navigate again and a new control gets thread 2 or 3, the exception always happens. That sound like the cleanup does not work properly.
• I have noticed that GlesContext.Dispose is executed from Unloaded handler of the control - that might be the cause of the problem, as Unloaded is always executed from the UI thread, so cleanup does not run on the same thread as the rendering.
• Noticed the Egl.eglMakeCurrent is never unset - eg. not called with EGL_NO_SURFACE etc. From here it looks like it should be done before the other resources are disposed of.
• Also on this docs page it seems that the context is not actually destroyed until it is no longer current of any thread. That seems to reinforce that suspicion that when the thread is reused, "current" is still set, so trying to call eglMakeCurrent again causes this exception.

I tried to add a flag cleanupContext to AngleSwapChainPanel and then I do the following in RenderLoop method:

private void RenderLoop(IAsyncAction action)
{
	while (action.Status == AsyncStatus.Started)
	{
		if (DrawInBackground)
		{
			if (cleanupContext)
			{
				glesContext?.Dispose();
				glesContext = null;
				break;
			}

			// run on this background thread
			RenderFrame();
		}
		else
		{
			// run in the main thread, block this one
			Dispatcher.RunAsync(CoreDispatcherPriority.Normal, RenderFrame).AsTask().Wait();
		}
	}
}

Then in Dispose of GlesContext I added:

Egl.eglMakeCurrent(eglDisplay, Egl.EGL_NO_SURFACE, Egl.EGL_NO_SURFACE, Egl.EGL_NO_CONTEXT);

That seems to help a bit, and I am actually sometimes able to navigate five times without the app crashing, but then it just suddenly completely freezes and looks like some kind of deadlock or something 😂 .

Also this workaround I tried does not solve the problem for RenderOnce - in that case the IAsyncAction is already null when Unloaded is called, so it is not possible to schedule the disposal on the background thread, as I don't have access to it anymore.

Do you have some ideas how to proceed please?

[taublast]

This issue affects SkiaSharp v3 on WinUI as well.

When you navigate to another page it can be that SKGLView handler resources are being disposed (especially when MAUI Shell disconnects previous content handlers upon navigation, a killer if you ask me).
This can happen in the middle of the rendering = crash.
This is caused by this line:

SkiaSharp/source/SkiaSharp.Views/SkiaSharp.Views.WinUI/SKSwapChainPanel.cs

Line 73 in cefe0e6

surface?.Dispose();

Will just leave a custom handler here for a quick workaround, notice it will not support SkiaSharp touch gestures as they are sealed:

//consume at init:
handlers.AddHandler(typeof(YourSKGLViewSubclassed), typeof(SKGLViewHandlerSafe));

//handler and view:

using System.Collections.Concurrent;
using SkiaSharp.Views.Maui.Handlers;
using SkiaSharp.Views.Windows;
using Rect = Windows.Foundation.Rect;
using SKPaintGLSurfaceEventArgs = SkiaSharp.Views.Maui.SKPaintGLSurfaceEventArgs;

namespace YourHandlersAndViews
{

    public partial class SKGLViewHandlerSafe : SKGLViewHandler
    {
        protected override SKSwapChainPanel CreatePlatformView()
        {
            return new SKSwapChainPanelFixed();
        }
    }

    public class SKSwapChainPanelFixed : SKSwapChainPanel
    {
        public bool IgnorePixelScaling { get; set; }
        public new SKSize CanvasSize => lastSize;
        public new GRContext GRContext => context;
        private const SKColorType colorType = SKColorType.Rgba8888;
        private const GRSurfaceOrigin surfaceOrigin = GRSurfaceOrigin.BottomLeft;
        private GRGlInterface glInterface;
        private GRContext context;
        private GRGlFramebufferInfo glInfo;
        private GRBackendRenderTarget renderTarget;
        private SKSurface surface;
        private SKSizeI lastSize;
        private bool cleanupRunning;
        private readonly ConcurrentBag<SKSurface> trashBag = new();

        public SKSwapChainPanelFixed()
        {
            cleanupRunning = true;
            Application.Current?.Dispatcher?.StartTimer(TimeSpan.FromSeconds(2), () =>
            {
                DisposeSafely();

                return cleanupRunning;
            });
        }

        void DisposeSafely()
        {
            while (trashBag.TryTake(out var pooledSurface))
            {
                pooledSurface.Dispose();
                Debug.WriteLine("Disposed pooled surface safely");
            }
        }

        protected override void OnRenderFrame(Rect rect)
        {

            if (context == null)
            {
                glInterface = GRGlInterface.Create();
                context = GRContext.CreateGl(glInterface);
            }

            var newSize = new SKSizeI((int)rect.Width, (int)rect.Height);

            bool needNewSurfaces = surface == null || lastSize != newSize;

            if (lastSize != newSize || renderTarget == null || !renderTarget.IsValid)
            {
                var previousTarget = renderTarget;
                if (previousTarget != null) //fix crash navigating into page
                {
                    Application.Current?.Dispatcher?.StartTimer(TimeSpan.FromSeconds(2), () =>
                    {
                        previousTarget.Dispose();
                        return false;
                    });
                }

                OpenGles.GetIntegerv(OpenGles.GL_FRAMEBUFFER_BINDING, out var framebuffer);
                OpenGles.GetIntegerv(OpenGles.GL_STENCIL_BITS, out var stencil);
                OpenGles.GetIntegerv(OpenGles.GL_SAMPLES, out var samples);
                var maxSamples = context.GetMaxSurfaceSampleCount(colorType);
                samples = Math.Min(samples, maxSamples);
                var glInfo = new GRGlFramebufferInfo((uint)framebuffer, colorType.ToGlSizedFormat());

                renderTarget =
                    new GRBackendRenderTarget(newSize.Width, newSize.Height, samples, stencil, glInfo);

                needNewSurfaces = true;
            }

            if (needNewSurfaces)
            {
                if (surface != null)
                {
                    // destroy the old surface
                    Debug.WriteLine("Abandoned surface to pool");
                    trashBag.Add(surface);
                    surface = null;
                }

                surface = SKSurface.Create(context, renderTarget, surfaceOrigin, colorType);
                Debug.WriteLine("Created new surface");

                lastSize = newSize;
            }

            // clear everything
            OpenGles.Clear(OpenGles.GL_COLOR_BUFFER_BIT | OpenGles.GL_DEPTH_BUFFER_BIT | OpenGles.GL_STENCIL_BUFFER_BIT);

            using (new SKAutoCanvasRestore(surface.Canvas, true))
            {
                // start drawing
                OnPaintSurface(new(surface, renderTarget, surfaceOrigin, colorType));
            }

            // update the control
            surface.Flush();
            context.Flush();
        }

        protected override void OnPaintSurface(SkiaSharp.Views.Windows.SKPaintGLSurfaceEventArgs e)
        {
            if (IgnorePixelScaling)
            {
                var density = (float)ContentsScale;
                var userVisibleSize = new SKSizeI((int)(e.Info.Width / density), (int)(e.Info.Height / density));
                var canvas = e.Surface.Canvas;
                canvas.Scale(density);
                canvas.Save();

                e = new(e.Surface, e.BackendRenderTarget, e.Origin, e.Info.WithSize(userVisibleSize), e.Info);
            }

            base.OnPaintSurface(e);
        }

        protected override void OnDestroyingContext()
        {
            base.OnDestroyingContext();

            renderTarget?.Dispose();

            cleanupRunning = false;

            surface?.Dispose();
            surface = null;

            context?.AbandonContext(false);
            context?.Dispose();
            context = null;

            glInterface?.Dispose();
            glInterface = null;

            lastSize = default;

            Application.Current?.Dispatcher?.StartTimer(TimeSpan.FromSeconds(2), () =>
            {
                DisposeSafely();

                return true;
            });
        }
    }

}

And You would need a wrapper of a sealed SkiaSharp class for that: taublast/DrawnUi@319dfb3#diff-23c5a4c0b082b4ffddb1262b5dfc9bbafef2fba9677bc9b0b2d9a2cb628c0df9

[taublast]

@mattleibow Could create a pr if you find this solution appropriate, or maybe there is another more elegant way to solve this.
EDIT: Must be tired, this is the workaround for a crash when navigating AWAY FROM a page with SKGLView. No idea if it's related, might be.
RE-EDIT: Added code to fix crash when "NAVIGATING TO". :)

[taublast]

Okay funny enough just catched it, crash when NAVIGATING INTO after changing some global internal stuff inside the App. This one was due to renderTarget becoming invalid, then disposed. Updated the workaround for this.

[mattleibow]

Could you explain the fix a bit? Looks like you got an infinite loop of disposals? You are basically creating a new surface and putting the old surface into the trash. This is all to avoid immediately disposing the surface?

What happens if you don't do anything and just let the GC collect it?

Does the trash have more than one item sometimes? Or is it just the previous one?

I wonder if you call flush / submit / synchronous on the surface before disposing? This will force the surface to be finished drawing.