Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fatal error. System.AccessViolationException When Encode #2595

Closed
4 tasks done
wuyu8512 opened this issue Nov 29, 2023 · 14 comments · Fixed by #2599
Closed
4 tasks done

Fatal error. System.AccessViolationException When Encode #2595

wuyu8512 opened this issue Nov 29, 2023 · 14 comments · Fixed by #2599
Assignees

Comments

@wuyu8512
Copy link

wuyu8512 commented Nov 29, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp
  • I have verified if the problem exist in both DEBUG and RELEASE mode
  • I have searched open and closed issues to ensure it has not already been reported

ImageSharp version

3.0.2

Other ImageSharp packages and versions

No

Environment (Operating system, version and so on)

Ubuntu 22.04.3 LTS

.NET Framework version

.Net 8

Description

info: Microsoft.Hosting.Lifetime[14]
      Now listening on: http://[::]:8080
info: Microsoft.Hosting.Lifetime[0]
      Application started. Press Ctrl+C to shut down.
info: Microsoft.Hosting.Lifetime[0]
      Hosting environment: Production
info: Microsoft.Hosting.Lifetime[0]
      Content root path: /app
Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at SixLabors.ImageSharp.Formats.Jpeg.Components.Encoder.SpectralConverter`1[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].ConvertStride(Int32)
   at SixLabors.ImageSharp.Formats.Jpeg.JpegEncoderCore.Encode[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](SixLabors.ImageSharp.Image`1<SixLabors.ImageSharp.PixelFormats.Bgra32>, System.IO.Stream, System.Threading.CancellationToken)
   at SixLabors.ImageSharp.Formats.ImageEncoder.<EncodeWithSeekableStreamAsync>g__DoEncodeAsync|8_0[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](System.IO.Stream, <>c__DisplayClass8_0`1<SixLabors.ImageSharp.PixelFormats.Bgra32> ByRef)
   at SixLabors.ImageSharp.Formats.ImageEncoder+<EncodeWithSeekableStreamAsync>d__8`1[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].MoveNext()
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[SixLabors.ImageSharp.Formats.ImageEncoder+<EncodeWithSeekableStreamAsync>d__8`1[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](<EncodeWithSeekableStreamAsync>d__8`1<SixLabors.ImageSharp.PixelFormats.Bgra32> ByRef)
   at SixLabors.ImageSharp.Formats.ImageEncoder.EncodeWithSeekableStreamAsync[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](SixLabors.ImageSharp.Image`1<SixLabors.ImageSharp.PixelFormats.Bgra32>, System.IO.Stream, System.Threading.CancellationToken)
   at Program+<>c__DisplayClass0_0+<<<Main>$>b__0>d.MoveNext()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1+AsyncStateMachineBox`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[Program+<>c__DisplayClass0_0+<<<Main>$>b__0>d, Image, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].MoveNext(System.Threading.Thread)
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Runtime.CompilerServices.IAsyncStateMachineBox, Boolean)
   at System.Threading.Tasks.Task.RunContinuations(System.Object)
   at SixLabors.ImageSharp.Image+<LoadAsync>d__70`1[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].MoveNext()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1+AsyncStateMachineBox`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[SixLabors.ImageSharp.Image+<LoadAsync>d__70`1[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].MoveNext(System.Threading.Thread)
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Runtime.CompilerServices.IAsyncStateMachineBox, Boolean)
   at System.Threading.Tasks.Task.RunContinuations(System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].SetResult(System.__Canon)
   at SixLabors.ImageSharp.Image+<WithSeekableStreamAsync>d__88`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1+AsyncStateMachineBox`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[SixLabors.ImageSharp.Image+<WithSeekableStreamAsync>d__88`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].MoveNext(System.Threading.Thread)
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Runtime.CompilerServices.IAsyncStateMachineBox, Boolean)
   at System.Threading.Tasks.Task.RunContinuations(System.Object)
   at SixLabors.ImageSharp.Formats.ImageDecoder+<DecodeAsync>d__2`1[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].MoveNext()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1+AsyncStateMachineBox`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[SixLabors.ImageSharp.Formats.ImageDecoder+<DecodeAsync>d__2`1[[SixLabors.ImageSharp.PixelFormats.Bgra32, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].MoveNext(System.Threading.Thread)
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Runtime.CompilerServices.IAsyncStateMachineBox, Boolean)
   at System.Threading.Tasks.Task.RunContinuations(System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].SetResult(System.__Canon)
   at SixLabors.ImageSharp.Formats.ImageDecoder+<CopyToMemoryStreamAndActionAsync>d__13`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1+AsyncStateMachineBox`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[SixLabors.ImageSharp.Formats.ImageDecoder+<CopyToMemoryStreamAndActionAsync>d__13`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].MoveNext(System.Threading.Thread)
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Runtime.CompilerServices.IAsyncStateMachineBox, Boolean)
   at System.Threading.Tasks.Task.RunContinuations(System.Object)
   at System.IO.Strategies.BufferedFileStreamStrategy+<CopyToAsyncCore>d__57.MoveNext()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1+AsyncStateMachineBox`1[[System.Threading.Tasks.VoidTaskResult, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.IO.Strategies.BufferedFileStreamStrategy+<CopyToAsyncCore>d__57, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext(System.Threading.Thread)
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Runtime.CompilerServices.IAsyncStateMachineBox, Boolean)
   at System.Threading.Tasks.Task.RunContinuations(System.Object)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()
   at System.IO.Stream+<<CopyToAsync>g__Core|27_0>d.MoveNext()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1[[System.Int64, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].SignalCompletion()
   at Microsoft.Win32.SafeHandles.SafeFileHandle+ThreadPoolValueTaskSource.ExecuteInternal()
   at Microsoft.Win32.SafeHandles.SafeFileHandle+ThreadPoolValueTaskSource.System.Threading.IThreadPoolWorkItem.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()

Steps to Reproduce

using SixLabors.ImageSharp;
using SixLabors.ImageSharp.Advanced;
using SixLabors.ImageSharp.PixelFormats;
using System.Runtime.CompilerServices;

var builder = WebApplication.CreateSlimBuilder(args);

var app = builder.Build();

var imgaeBasePath = app.Configuration.GetSection("Image")["BasePath"].ToString();

app.MapGet("{hash:length(8)}/{user:length(4)}/{**path}", async (string hash, string user, string path) =>
{
    var filePath = Path.Combine(imgaeBasePath, path);
    using var inputImage = await SixLabors.ImageSharp.Image.LoadAsync<Bgra32>(filePath);

    var pixel = new byte[inputImage.Height * inputImage.Width * Unsafe.SizeOf<Bgra32>()];
    inputImage.CopyPixelDataTo(pixel);

    using var outputImage = SixLabors.ImageSharp.Image.WrapMemory<Bgra32>(pixel, inputImage.Width, inputImage.Height);

    var stream = new MemoryStream();
    var format = inputImage.Metadata.DecodedImageFormat;

    switch (format)
    {
        case SixLabors.ImageSharp.Formats.Jpeg.JpegFormat:
            await outputImage.SaveAsJpegAsync(stream, new SixLabors.ImageSharp.Formats.Jpeg.JpegEncoder() { Quality = 95 });
            break;
        case SixLabors.ImageSharp.Formats.Webp.WebpFormat:
            await outputImage.SaveAsWebpAsync(stream, new SixLabors.ImageSharp.Formats.Webp.WebpEncoder() { Quality = 95 });
            break;
        default:
            await outputImage.SaveAsync(stream, inputImage.GetConfiguration().ImageFormatsManager.GetEncoder(format));
            break;
    }

    stream.Position = 0;
    return Results.File(stream, format.DefaultMimeType);
});

app.Run();

Sorry for my bad English,
When big number of requests(about avg 2request/1s), an error will throw, But I can't reproduce it on my local computer

Images

No response

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Nov 29, 2023

We’ll need an input image that triggers the crash.

This is very odd code btw? Why are you creating that byte[] and copying to it?

@wuyu8512
Copy link
Author

wuyu8512 commented Nov 30, 2023

This error does not appear in the same picture, but occurs randomly in tens of thousands of pictures.
Many steps are omitted here. Even without these steps, this code will generate errors.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Nov 30, 2023

@br3aker I'm a little confused following this code so will need your assistance diagnosing if I can?

I'm assuming the unused property paddedPixelsCount is meaningless? I'm not sure how we can go out of bounds here given that the lanes are created as a multiple of that width.

image

@wuyu8512 Are you operating on image or outImage? I'm still struggling to understand why you are wrapping memory, the buffer never gets set to null does it?

If you need direct pixel access for your operations. This is the best and secure way.

image.ProcessPixelRows(accessor =>
{
	for (int y = 0; y < accessor.Height; y++)
	{
		Span<Bgra32> row = accessor.GetRowSpan(y);
		Span<byte> byteRow = MemoryMarshal.Cast<Bgra32, byte>(row);
		
		// Manipulate the row bytes directly.			
	}
});

@wuyu8512
Copy link
Author

wuyu8512 commented Nov 30, 2023

@wuyu8512 Are you operating on image or outImage?

i was operating on image

I'm still struggling to understand why you are wrapping memory

I hope to be compatible with both ImageSharp and SkiaSharp, so getting all the data and processing it will make it easier for me to write a universal library

the buffer never gets set to null does it?

Yes, I updated all the code

Assuming that the object of the Save method is changed to inputImage object, the error will not occur, so it is suspected that something unexpected happened to WrapMemory.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Nov 30, 2023

Save doesn't mutate the image. It only encodes it.

I can't see where pixels is set to null in your sample. I'm assuming then that you are passing the pixels buffer in its entirety to your custom filter methods to operate upon before saving?

WrapMemory depends on the buffer being available and its dimensions unchanged for the lifetime of the operation. Any change there will lead to issues.

You should create a 2D buffer abstraction that allows per-row access instead of using the array for your custom methods. That would be the only type you implement for both ImageSharp and SkiaSharp.

@wuyu8512
Copy link
Author

wuyu8512 commented Nov 30, 2023

Save doesn't mutate the image. It only encodes it.

Sorry, my express was not good, I modify my express again

I can't see where pixels is set to null in your sample. I'm assuming then that you are passing the pixels buffer in its entirety to your custom filter methods to operate upon before saving?

Yes, the pixels buffer will never set null, This code can be reproduced directly

You should create a 2D buffer abstraction that allows per-row access instead of using the array for your custom methods. That would be the only type you implement for both ImageSharp and SkiaSharp.

i will try

@wuyu8512
Copy link
Author

wuyu8512 commented Nov 30, 2023

After forcing the use of Webp, the problem no longer occurs

var filePath = Path.Combine(imgaeBasePath, path);
using var inputImage = await SixLabors.ImageSharp.Image.LoadAsync<Bgra32>(filePath);

var pixel = new byte[inputImage.Height * inputImage.Width * Unsafe.SizeOf<Bgra32>()];
inputImage.CopyPixelDataTo(pixel);

using var outputImage = SixLabors.ImageSharp.Image.WrapMemory<Bgra32>(pixel, inputImage.Width, inputImage.Height);

var stream = new MemoryStream();

await outputImage.SaveAsWebpAsync(stream, new SixLabors.ImageSharp.Formats.Webp.WebpEncoder() { Quality = 95 });

stream.Position = 0;
return Results.File(stream, SixLabors.ImageSharp.Formats.Webp.WebpFormat.Instance.DefaultMimeType);

@JimBobSquarePants
Copy link
Member

I suspect that WrapMemory is not related to the issue at all but just to check, can you see if you can replicate using JpegEncoder only and no wrapping?

If so, please let me know and also let me know the dimension range of the images you are using?

@wuyu8512
Copy link
Author

wuyu8512 commented Nov 30, 2023

if use LoadPixelData Replace WrapMemory , Problem no longer recurs
my image are most around 1000*1500 and 500 * 750(md version)

Since this issue only occurs in my production environment, I'm sorry I can't always test it

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Nov 30, 2023

Is this sample (but using the jpeg encoder) enough to trigger the issue?

#2595 (comment)

@wuyu8512
Copy link
Author

Is this sample (but using the jpeg encoder) enough to trigger the issue?

yes

@antonfirsov
Copy link
Member

antonfirsov commented Nov 30, 2023

Isn't it the source that has to be padded by 3 bytes actually?

/// Bulk operation that unpacks pixels from <paramref name="source"/>
/// into 3 seperate RGB channels. The destination must have a padding of 3.
/// </summary>
/// <param name="redChannel">A <see cref="ReadOnlySpan{T}"/> to the red values.</param>
/// <param name="greenChannel">A <see cref="ReadOnlySpan{T}"/> to the green values.</param>
/// <param name="blueChannel">A <see cref="ReadOnlySpan{T}"/> to the blue values.</param>
/// <param name="source">A <see cref="Span{T}"/> to the destination pixels.</param>
internal virtual void UnpackIntoRgbPlanes(
Span<float> redChannel,
Span<float> greenChannel,
Span<float> blueChannel,
ReadOnlySpan<TPixel> source)

I need to refresh my memory about the SIMD packing/unpacking algorithms, but I would think this could be a more likely case, assuming a83b3b6 from #2120 implemented the mirror operation of #1462.

In this case UnpackIntoRgbPlanes would need to operate on a copy of sourceRow when processing the last row:

PixelOperations<TPixel>.Instance.UnpackIntoRgbPlanes(rLane, gLane, bLane, sourceRow);

This is how it's done in the decoder:

// PackFromRgbPlanes expects the destination to be padded, so try to get padded span containing extra elements from the next row.
// If we can't get such a padded row because we are on a MemoryGroup boundary or at the last row,
// pack pixels to a temporary, padded proxy buffer, then copy the relevant values to the destination row.
if (this.pixelBuffer.DangerousTryGetPaddedRowSpan(yy, 3, out Span<TPixel> destRow))
{
PixelOperations<TPixel>.Instance.PackFromRgbPlanes(r, g, b, destRow);
}
else
{
Span<TPixel> proxyRow = this.paddedProxyPixelRow.GetSpan();
PixelOperations<TPixel>.Instance.PackFromRgbPlanes(r, g, b, proxyRow);
proxyRow[..width].CopyTo(this.pixelBuffer.DangerousGetRowSpan(yy));
}

@antonfirsov
Copy link
Member

I can repro this without WrapMemory:

Configuration.Default.MemoryAllocator = new SimpleGcMemoryAllocator();
Parallel.For(0, 10_000, i =>
{
    using Image<Bgra32> image = new Image<Bgra32>(Random.Shared.Next(3000) + 1, Random.Shared.Next(3000) + 1);
    using MemoryStream stream = new MemoryStream();
    image.SaveAsJpeg(stream);
});

@antonfirsov antonfirsov self-assigned this Nov 30, 2023
@JimBobSquarePants
Copy link
Member

This can be further reduced to

Configuration configuration = Configuration.CreateDefaultInstance();
configuration.MemoryAllocator = new SimpleGcMemoryAllocator();
using Image<Bgra32> image = new(configuration, 132, 1606);
using MemoryStream stream = new();
image.SaveAsJpeg(stream);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants