Thread Safety in SKBitmap.Decode?

[abezydar]

Hi! I'm using SkiaSharp for server side image generation and have run into an issue when Decoding many bitmaps at once on IIS.

Here's the error:

Application: w3wp.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.AccessViolationException
   at SkiaSharp.SkiaApi.sk_codec_get_info(IntPtr, SkiaSharp.SKImageInfo ByRef)
   at SkiaSharp.SKBitmap.Decode(SkiaSharp.SKCodec)
   at SkiaSharp.SKBitmap.Decode(SkiaSharp.SKStream)

I'm not sure if this is because of memory constraints, because of thread safety or another reason I'm unaware of.

The code I have is this:

public static async Task LoadBitmap(RenderContext context, ILayoutElementHasUri element, Action<SKBitmap> handleBitmapAction) {
    
    var uri = element.GetUriWithReplacements();

    var bytes = await new LoadImage(uri, context).Execute();

    // Limit the number of bitmaps decoding concurrently
    await decodeBitmapThrottler.WaitAsync();

    try
    {
        var bitmap = SKBitmap.Decode(bytes);
        handleBitmapAction(bitmap);
        if (bitmap != null)
        {
            bitmap.Dispose();
        }
        
    }
    finally
    {
        decodeBitmapThrottler.Release();
    }
}

My workaround has been to add a SemaphoreSlim, decodeBitmapThrottler, with a max count of 1, which increases the stability but sacrifices speed. As soon as I increase the count, I start getting System.AccessViolationException errors.

Any help or insight is appreciated! I also understand if this case is outside the goals of the project.

VS bug #735689

[mattleibow]

Hi @abezydar, thanks for giving SkiaSharp a go.

I am having a look at your code and I can't really see why threads would come into this. There is just the single SKBitmap.Decode operation. the bytes variable - is that a byte array or is it a stream (as appears by the stack trace). If it is a stream, is there a way to test and see if you load the stream into a byte array and see if the issue goes away? (this is not a fix, but just checking to see where the error is)

SkiaSharp is "thread safe" in the sense that you can have two threads loading a bitmap, but two threads shouldn't access the same object.

[abezydar]

Hey @mattleibow, no problem and thanks for the feedback. The bytes variable is a byte array. It's multithreaded since an API in IIS is used to call this method. In the load test, 20 clients are decoding images at the same time. So, my expectation is that IIS is using multiple threads in the same worker process to decode the images.

Since the error is happening on the SKBitmap.Decode method, my thought was it couldn't handle decoding more than one byte array at a time reliably. If it should, then perhaps I'm running into a memory error? If that's the case, I can look into limiting access to SKBitmap.Decode by memory availability rather than concurrent processes.

[mattleibow]

I am going to re-investigate this. Here is another example of this issue: https://forums.xamarin.com/discussion/87687/usage-in-visual-studio-2015-unit-test-project

[mattleibow]

This is the code that fails:

public class ImagingUnitTest
{
    public static void ImageScalingMultipleThreads()
    {
        const int numThreads = 100;
        const int numIterationsPerThread = 1000;

        var referenceFile = @"/Users/matthew/Documents/baboon.jpg";

        var tasks = new List<Task>();

        for (int i = 0; i < numThreads; i++)
        {
            var task = Task.Run(() =>
            {
                for (int j = 0; j < numIterationsPerThread; j++)
                {
                    var imageData = ComputeThumbnail(referenceFile);
                }
            });
            tasks.Add(task);
        }

        Task.WaitAll(tasks.ToArray());

        Console.WriteLine($"Test completed for {numThreads} tasks, {numIterationsPerThread} each.");
    }

    public static byte[] ComputeThumbnail(string fileName)
    {
        using (var ms = new MemoryStream())
        using (var bitmap = SKBitmap.Decode(fileName))
        using (var scaledBitmap = new SKBitmap(60, 40, bitmap.ColorType, bitmap.AlphaType))
        {
            SKBitmap.Resize(scaledBitmap, bitmap, SKBitmapResizeMethod.Hamming);

            using (var image = SKImage.FromBitmap(scaledBitmap))
            using (var data = image.Encode(SKImageEncodeFormat.Png, 80))
            {
                data.SaveTo(ms);

                return ms.ToArray();
            }
        }
    }
}

[abezydar]

Thanks for looking into it @mattleibow!

[groege]

Is there any workarount?

[mattleibow]

I am not sure what I did to fix this :) It may have been an issue with skia returning the same instance of the decoders, and me not handling those cases. I think the fix is that I now track each time skia gives me an instance, and make sure it is not in use before disposing.

I am cleaning up / preparing for a release - hopefully this week.

[abezydar]

Cool, I'll check it out as soon as the release comes out! Thanks!

[ncthbrt]

@mattleibow any updates on this? I'm running into a similar problem in netcore in version 1.56.2 Code is in this repo: https://github.com/ncthbrt/skia-fsharp

This is where the drawing call is made: https://github.com/ncthbrt/skia-fsharp/blob/master/Drawing/Test.fs#L94

Unhandled Exception: System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at System.Buffer.__Memmove(Byte* dest, Byte* src, UInt64 len)
   at System.Buffer._Memmove(Byte* dest, Byte* src, UInt64 len)
   at System.Buffer.Memmove(Byte* dest, Byte* src, UInt64 len)
   at System.Buffer.Memcpy(Byte[] dest, Int32 destIndex, Byte* src, Int32 srcIndex, Int32 len)
   at System.IO.UnmanagedMemoryStream.Read(Byte[] buffer, Int32 offset, Int32 count)
   at System.IO.UnmanagedMemoryStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellation
Token)

[mattleibow]

Moving to the next release so we can get the new skia out.

[tdoneal]

Hi, I'm having similar issues with 1.56.2. I'm getting nondeterministic AccessViolationExceptions using Windows Forms. Strangely, the problem occurs only in Release mode (maybe due to tighter memory packing?). Anyway, I'm pretty sure I'm allocating my surface correctly:

        using (var bitmap = new Bitmap(iw, ih, PixelFormat.Format32bppPArgb))
        {
            var data = bitmap.LockBits(new Rectangle(0, 0, iw, ih), ImageLockMode.WriteOnly, bitmap.PixelFormat);

            var surface = SKSurface.Create(iw, ih, SKColorType.Bgra8888, SKAlphaType.Premul, data.Scan0, iw * 4);
            var skcanvas = surface.Canvas;
            .... // canvas.draw(), etc

This was working very reliably for months while in Debug mode, but switching to Release mode caused it to start failing. Is there a bug in Skia in handling the allocation, or am I missing something silly? There's no other place in my code where I'm doing "low level" memory management so I don't really know how to go about fixing this. I commented out the actual canvas.draw() calls but I'm still getting the AccessViolationExceptions. I can try to get a minimal reproducing test case if the above code is indeed correct.

[tdoneal]

Small correction: commenting out the actual canvas.draw() calls causes the exceptions to disappear. Let me know if you have any debugging tips to try.

[tdoneal]

Update: apparently I wasn't disposing of the SKSurface properly. I wasn't calling surface.Dispose() after calling bitmap.UnlockBits(). Adding the call to surface.Dispose() fixed the issue. I'll leave the above comments here in case anyone else runs into the same thing.

[mattleibow]

Glad to hear that you got it to work.

I would say that the "best practice" is to wrap SkiaSharp types in a using statement. This will make sure that there are no memory leaks as well as all the memory is freed ASAP. One thing where this helps is with the bitmap types, the managed type has only a size of IntPtr, but the underlying data could be several megabytes.

[ncthbrt]

@mattleibow this appears to be still an issue. I think @tdoneal had a different problem

[samirchakour-crossover]

I am having the same issue on Android, on iOS its working fine.

_blobBitmap = SKBitmap.Decode(filePath);

After the call to SKBitmap.Decode the app crash with a memory violation issue.

Here is my project.json file on Android :

<package id="SkiaSharp" version="1.57.0" targetFramework="monoandroid71" /> <package id="SkiaSharp.Views" version="1.57.0" targetFramework="monoandroid71" /> <package id="SkiaSharp.Views.Forms" version="1.57.0" targetFramework="monoandroid71" />

Here is the crash log : logcat-android-skiasharp-crash.txt

[samirchakour-crossover]

I have done a couple of changes to my android configuration and the bug seems to be resolved for me :

1. Changed the targetFramework to 'monoandroid60' instead of 'monoandroid71'
2. Changed the link behaviour to "Don't Link"
3. Disabled the arm64 and x86_64 ABIs

And SKBitmap.Decode is working now, I haven't investigated yet which of those changes resolve the issue but will do that on the next few days.

Hope this will help others.

[ncthbrt]

I was thinking... Would it be possible that aspnet core is interfering with GC?

[irv]

I'm also hitting this (in dotnet core, running on .net 4.6.1)

it appears to pop here: https://github.com/mono/SkiaSharp/blob/1cc9af5adb5a9966361ac5a4138952694a5345d3/binding/Binding/SKImage.cs#L59

from the stacktrace:

at SkiaSharp.SkiaApi.sk_image_unref(IntPtr image) at SkiaSharp.SKImage.Dispose(Boolean disposing) at SkiaSharp.SKNativeObject.Dispose()

It looks like maybe Dispose() is being called twice?

[mattleibow]

This is something that we will be looking at in the future.

[christian289]

Hello. I am currently debugging in Windows 10 VisualStudio 2019 environment using SkiaSharp.NativeAssets.Linux.NoDependencies version 2.80.3. We are debugging into a .NET 5 environment in the Azure Function Isolate Container Project, and we will finally release it to Azure Function. I am currently experiencing the same symptoms as the problem in this thread.

My calling sequence is as below code.

        private ReadOnlySpan<byte> DownloadImageToSpan(HttpClient httpClient, Uri imageUri, bool tryResize = false)
        {
            byte[] imagebytes = null;

            try
            {
                imagebytes = httpClient.GetByteArrayAsync(imageUri).Result;
            }
            catch
            {
                throw;
            }

            if (imagebytes.Length == 0)
            {
                return null;
            }
            return imagebytes.AsSpan();
        }

            ReadOnlySpan<byte> downloadSpan = DownloadImageToSpan(httpClient, messageObj.SourceUri);
            SKBitmap decode = SKBitmap.Decode(downloadSpan); // System.AccessViolationException Occur!

The stack trace is below.

[2021-10-27T01:45:05.139Z] Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt. [2021-10-27T01:45:05.142Z] Repeat 2 times: [2021-10-27T01:45:05.143Z] -------------------------------- [2021-10-27T01:45:05.146Z] at SkiaSharp.SkiaApi.sk_codec_get_pixels(IntPtr, SkiaSharp.SKImageInfoNative*, Void*, IntPtr, SkiaSharp.SKCodecOptionsInternal*) [2021-10-27T01:45:05.148Z] -------------------------------- [2021-10-27T01:45:05.149Z] at SkiaSharp.SKCodec.GetPixels(SkiaSharp.SKImageInfo, IntPtr, Int32, SkiaSharp.SKCodecOptions) [2021-10-27T01:45:05.151Z] at SkiaSharp.SKCodec.GetPixels(SkiaSharp.SKImageInfo, IntPtr) [2021-10-27T01:45:05.152Z] at SkiaSharp.SKBitmap.Decode(SkiaSharp.SKCodec, SkiaSharp.SKImageInfo) [2021-10-27T01:45:05.153Z] at SkiaSharp.SKBitmap.Decode(SkiaSharp.SKCodec) [2021-10-27T01:45:05.155Z] at SkiaSharp.SKBitmap.Decode(System.ReadOnlySpan1<Byte>) [2021-10-27T01:45:05.156Z] at ServiceBusFunc.Functions.ImageAutoSlice.Run(System.String, Microsoft.Azure.Functions.Worker.FunctionContext) [2021-10-27T01:45:05.158Z] at DynamicClass.lambda_method6(System.Runtime.CompilerServices.Closure, ServiceBusFunc.Functions.ImageAutoSlice, System.Object[]) [2021-10-27T01:45:05.162Z] at Microsoft.Azure.Functions.Worker.Invocation.VoidMethodInvoker2[[System.__Canon, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].InvokeAsync(System.__Canon, System.Object[]) [2021-10-27T01:45:05.165Z] at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionInvoker2[[System.__Canon, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.__Canon, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].InvokeAsync(System.Object, System.Object[]) [2021-10-27T01:45:05.167Z] at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionExecutor+<ExecuteAsync>d__4.MoveNext() [2021-10-27T01:45:05.169Z] at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionExecutor+<ExecuteAsync>d__4, Microsoft.Azure.Functions.Worker.Core, Version=1.3.1.0, Culture=neutral, PublicKeyToken=551316b6919f366c]](<ExecuteAsync>d__4 ByRef) [2021-10-27T01:45:05.172Z] at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionExecutor.ExecuteAsync(Microsoft.Azure.Functions.Worker.FunctionContext) [2021-10-27T01:45:05.174Z] at Microsoft.Azure.Functions.Worker.OutputBindings.OutputBindingsMiddleware+<Invoke>d__0.MoveNext() [2021-10-27T01:45:05.177Z] at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Azure.Functions.Worker.OutputBindings.OutputBindingsMiddleware+<Invoke>d__0, Microsoft.Azure.Functions.Worker.Core, Version=1.3.1.0, Culture=neutral, PublicKeyToken=551316b6919f366c]](<Invoke>d__0 ByRef) [2021-10-27T01:45:05.179Z] at Microsoft.Azure.Functions.Worker.OutputBindings.OutputBindingsMiddleware.Invoke(Microsoft.Azure.Functions.Worker.FunctionContext, Microsoft.Azure.Functions.Worker.Middleware.FunctionExecutionDelegate) [2021-10-27T01:45:05.181Z] at Microsoft.Azure.Functions.Worker.FunctionsApplication.InvokeFunctionAsync(Microsoft.Azure.Functions.Worker.FunctionContext) [2021-10-27T01:45:05.183Z] at Microsoft.Azure.Functions.Worker.GrpcWorker+<InvocationRequestHandlerAsync>d__17.MoveNext() [2021-10-27T01:45:05.185Z] at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Azure.Functions.Worker.GrpcWorker+<InvocationRequestHandlerAsync>d__17, Microsoft.Azure.Functions.Worker.Grpc, Version=1.3.0.0, Culture=neutral, PublicKeyToken=551316b6919f366c]](<InvocationRequestHandlerAsync>d__17 ByRef) [2021-10-27T01:45:05.186Z] at Microsoft.Azure.Functions.Worker.GrpcWorker.InvocationRequestHandlerAsync(Microsoft.Azure.Functions.Worker.Grpc.Messages.InvocationRequest, Microsoft.Azure.Functions.Worker.IFunctionsApplication, Microsoft.Azure.Functions.Worker.IInvocationFeaturesFactory, Azure.Core.Serialization.ObjectSerializer, Microsoft.Azure.Functions.Worker.OutputBindings.IOutputBindingsInfoProvider) [2021-10-27T01:45:05.193Z] at Microsoft.Azure.Functions.Worker.GrpcWorker+<ProcessRequestCoreAsync>d__16.MoveNext() [2021-10-27T01:45:05.194Z] at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Azure.Functions.Worker.GrpcWorker+<ProcessRequestCoreAsync>d__16, Microsoft.Azure.Functions.Worker.Grpc, Version=1.3.0.0, Culture=neutral, PublicKeyToken=551316b6919f366c]](<ProcessRequestCoreAsync>d__16 ByRef) [2021-10-27T01:45:05.197Z] at Microsoft.Azure.Functions.Worker.GrpcWorker.ProcessRequestCoreAsync(Microsoft.Azure.Functions.Worker.Grpc.Messages.StreamingMessage) [2021-10-27T01:45:05.199Z] at System.Threading.Tasks.Task1[[System.__Canon, System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].InnerInvoke() [2021-10-27T01:45:05.201Z] at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(System.Threading.Thread, System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object) [2021-10-27T01:45:05.203Z] at System.Threading.Tasks.Task.ExecuteWithThreadLocal(System.Threading.Tasks.Task ByRef, System.Threading.Thread) [2021-10-27T01:45:05.207Z] at System.Threading.ThreadPoolWorkQueue.Dispatch()

As the people above mentioned, this also happens when I process multiple Images.

Ask for advice.

[christian289]

Oh, my acquaintance gave me the solution. Thank you. evan.

Here is the solution you gave me:

        private byte[] DownloadImageToByteArray(HttpClient httpClient, Uri imageUri, bool tryResize = false)
        {
            byte[] imagebytes = null;

            try
            {
                imagebytes = httpClient.GetByteArrayAsync(imageUri).Result;
            }
            catch
            {
                throw;
            }

            if (imagebytes.Length == 0)
            {
                return null;
            }

            return imagebytes;
        }

        private SKBitmap MakeSKBitmap(ReadOnlySpan<byte> span)
        {
            using MemoryStream ms = new(span.ToArray());
            using SKManagedStream skStream = new(ms, false);
            using SKCodec codec = SKCodec.Create(skStream);

            return SKBitmap.Decode(codec);
        }

        ReadOnlySpan<byte> downloadSpan = DownloadImageToByteArray(httpClient, messageObj.SourceUri);
        SKBitmap decode = MakeSKBitmap(downloadSpan); // No Exception!

I hope it will be of help.

[kostebudinoski]

Hello

is there any update on this?

We recently updated to latest version (2.88.5), and following issue started randomly popping up:

Application: dotnet.exe
CoreCLR Version: 7.0.923.32018
.NET Version: 7.0.9
Description: The process was terminated due to an unhandled exception.
Exception Info: System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Stack:
   at SkiaSharp.SkiaApi.sk_codec_new_from_stream(IntPtr, SkiaSharp.SKCodecResult*)
   at SkiaSharp.SkiaApi.sk_codec_new_from_stream(IntPtr, SkiaSharp.SKCodecResult*)
   at SkiaSharp.SKCodec.Create(SkiaSharp.SKStream, SkiaSharp.SKCodecResult ByRef)
   at SkiaSharp.SKBitmap.Decode(System.IO.Stream)

it crashes the whole process.

[kostebudinoski]

loading the stream into a byte array first, and than decoding it, makes the issue go away. @mattleibow what could be the reason for it? would prefer using the stream I have.

I think it's also worth mentioning that the image is read from azure blob storage and it's around 20mb in size.

The stream, that is being read, is of type: LazyLoadingReadOnlyStream<BlobProperties>

[kostebudinoski]

Reverting SkiaSharp to version 2.80.2 (what we used before the upgrade to 2.88.5) and the issue is gone.

I believe you now have ample investigative materials.

[gktval]

I am seeing this on an Asp.Net core application, but only in IIS. During debug on my computer it seems to work fine. I went back to version 2.80.2, but it still has the error. The encode process will run fine for a few times, but it will randomly happen after 4-8 'encode' calls.

here is the error from the log:

Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Repeat 2 times:
--------------------------------
   at SkiaSharp.SkiaApi.sk_pixmap_encode_image(IntPtr, IntPtr, SkiaSharp.SKEncodedImageFormat, Int32)
--------------------------------
   at SkiaSharp.SKPixmap.Encode(SkiaSharp.SKWStream, SkiaSharp.SKEncodedImageFormat, Int32)
   at SkiaSharp.SKPixmap.Encode(SkiaSharp.SKEncodedImageFormat, Int32)
   at SkiaSharp.SKBitmap.Encode(SkiaSharp.SKEncodedImageFormat, Int32)
   ...

seems to be crashing on this line of code:

var skData = bmp.Encode(SKEncodedImageFormat.Png, 10);

Edit: Fixed. In my code prior to this I was using a pointer like below.

 // pin the managed array so that the GC doesn't move it
 var handle = GCHandle.Alloc(outBuffer, GCHandleType.Pinned);
 bmp.InstallPixels(bmp.Info, handle.AddrOfPinnedObject());
 handle.Free();
 GC.Collect();   

Removing the GC.Collect() fixed the error

[Proektsoftbg]

@gktval . It is not enough just to remove GC.Collect()

If you need to use the bmp object further (e.g. to 'Encode'), you should not immediately call handle.Free(). The GC Handle must be released after the memory is no longer needed. Then, you can safely call handle.Free(), and definitely after bmp.Encode(). Otherwise, GC can "decide" to collect the array behind the handle meanwhile and will destroy your bitmap data. If you try to use this bitmap after that, you will get again "Attempted to read or write protected memory".

Removing GC.Collect() just makes it to not happen each time. Now, it can happen rarely, but will cause your app to crash. You can catch it if you simulate heavy load on your app. This will trigger GC more often and there will be a greater chance to hit the small time span between freeing the handle and trying to use the bitmap. This is some of those nasty hidden bugs that can squeeze through your tests and cause troubles to your clients.

[kostebudinoski]

Taking into consideration the current vulnerability highlighted at https://security.snyk.io/vuln/SNYK-DOTNET-SKIASHARP-5922114, updating to the latest version becomes even more challenging. With one memory issue on one side and another vulnerability on the other, the task is compounded. @mattleibow , any insights you can provide would be greatly appreciated.