Repository navigation
Access Violation in 2.1, works fine in 1.04 #2075
Description
Activity
Thanks @alex-jitbit for raising this.
However, without either a stack trace, dump or image we’re absolutely powerless to do anything about it. You’ll need to provide means to replicate the issue or submit a patch.
@JimBobSquarePants 100% understand. I will try to set up automated crash-dump saving, and then let it run on 2.1 for a while. If I won't be able to provide more details feel free to close.
Reacted by Anton FirszovThanks. We definitely won’t be closing this issue. It’s top priority to avoid memory access violations.
This may or may not be related but I have noticed we're missing a sanitation check here.
ImageSharp/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs
Lines 723 to 734 in 136cc14
We really should be checking for a valid length before reading despite how rare this should be.
if (ProfileResolver.IsProfile(this.temp, ProfileResolver.XmpMarker.Slice(0, ExifMarkerLength))) { const int remainingXmpMarkerBytes = XmpMarkerLength - ExifMarkerLength; if (remaining < remainingXmpMarkerBytes || this.IgnoreMetadata) { // Skip the application header length. stream.Skip(remaining); return; } stream.Read(this.temp, ExifMarkerLength, remainingXmpMarkerBytes);
@JimBobSquarePants thanks! Will try my best to dig some more debug info. I completely understand.
JFYI, this is the code we use, like I said it's very basic
using (Image srcBmp = Image.Load(imgData)) //"imgData" is a MemoryStream { //omitted some custom code that does not use ImageSharp (calculating target dimensions) //... srcBmp.Mutate(x => x.Resize(newWidth, newHeight)); //sometimes we use this instead of the above srcBmp.Mutate(x => x.Resize(new ResizeOptions { Mode = ResizeMode.Pad, Position = AnchorPositionMode.Center, Size = new Size(newWidth, newHeight), Sampler = KnownResamplers.NearestNeighbor })); using (var resultMs = new MemoryStream()) { srcBmp.Save(resultMs, IsJpeg(imgData) ? new JpegEncoder() : new PngEncoder()); //"IsJpeg" is our custom function that uses "magic numbers" to tell JPG from everything else } }
Reacted by James Jackson-South@alex-jitbit unrelated to the bug reported but thought you would like to know but there is an overload of
Image.Load(imgData, out IImageFormat imageFormat)that returns the format of the image loaded, saves you having to process theimgDatastream twice as we have already done exactly that during load. Also there is an overload/extension method onImagefor saving that takes in anIImageFormatso you can callsrcBmp.Save(resultMs, imageFormat);to save in the same format as the source image and avoid create a new encoder instance on each call. (at the very least you want to make them singletons, there is no need to create a new one every image).Reacted by James Jackson-South, Tieson Trowbridge and ShapehReacted by Alexander Yumashev@alex-jitbit any update on means of replicating? We’re blocked from any further development of the libraries until we get this sorted.
@JimBobSquarePants no, sorry, I was prohibited from setting up the automated crash dumps on production and can't replicate this on staging or local dev machine. I completely understand this issue is useless without STR, so feel free to close
Not even a try… catch around the relevant code to identify a problematic image? You don’t need a process dump.
@JimBobSquarePants we do have a try-catch around it, but no exception is thrown.
Damn.... That's nasty! And you definitely have the same version deployed on both staging and production with the same images?
@alex-jitbit corrupted state exceptions are not caught by try-catch by default, 'access violation' is one of them. Unfortunately, dotnet6 dropped support of the CSE exception handling but you can use this event:
AppDomain.UnhandledException.Problem is that process crash can be caused by various exceptions, so it's better to confirm exact exception type before watching for potential access violations.
Reacted by James Jackson-South@alex-jitbit Did you get permission to ad @br3aker suggested capture approach?
@alex-jitbit We've pushed some significant decoder sanitation code to main which is now in build
2.1.1-alpha.0.4which should fix any memory access issues.It would be great if you could test this release for us before we ship a release to NuGet
Just updated to 2.1.2 and let it run for 2 days. Today received another access violation
Application: w3wp.exe CoreCLR Version: 6.0.522.21309 .NET Version: 6.0.5 Description: The process was terminated due to an internal error in the .NET Runtime at IP 00007FFBF6DBA639 (00007FFBF6CD0000) with exit code 80131506.Reacted by Brian PopowIt’s more than a little disappointing that you took so long to get back to us. I’m not sure what you expect us to be able to do.
Reacted by Dmitry@JimBobSquarePants Sorry about that. It's OK I'll just stick to v1 then. JFYI it appears it's not related to some particular image, but to a high-load scenario with ~15k requests per minute. Must be the new pooling/pool trimming mechanisms.
It's OK I'll just stick to v1 then
@alex-jitbit Unfortunately that doesn't really help us, or anyone else who may encounter issues. Did you try the suggestion by @br3aker to gather more information?
@alex-jitbit you can't stay on 1.0 forever, so it would be great to give another try to figure this out :)
Is 2 days the actual up-time before your high-load service dies? What does your service do? (=what ImageSharp API-s are called).
@JimBobSquarePants I will add an AppDomain.UnhandledException handler and try to gather more info and get back to you. But I doubt it will help, since I run on Windows and it logs unhandled exceptions in Event Log anyway.
@antonfirsov here is the code we use. Our app a is CRUD business app (asp.net core) with file attachments, that simply generates thumbnails for UGC images before storing those on CDN.
I just realized that my app is published as
ReadyToRunso it might be connected to #2148Have you tried .NET 7?
@JimBobSquarePants unfortunately, no because it's STS, while most of enterprise-grade customers have policies that require LTS releases only
@alex-jitbit any further information you could share upon the fact that the load is ~15k RPM? (Typical formats, image sizes, hardware configuration etc.) Any chance you can create a crash dump?
Prerequisites
DEBUGandRELEASEmodeImageSharp version
2.1.0
Other ImageSharp packages and versions
none
Environment (Operating system, version and so on)
Windows 2016
.NET Framework version
6.0
Description
We have an ASP.NET 6 app that uses ImageSharp 2.1 to resize images and create thumbnails (and that's it) for UGC. Several times a day, during high-load, our app crashes with this error:
It's very simple code. But this uncatchable error crashes our entire process.
Reverting back to 1.0.4 seems to help.
Steps to Reproduce
Unfortunately I'm unable to reliably tell which images caused this, since it's a busy app with millions of HTTP requests. Also I had no time to create process dumps or anything, But I did narrow this down to ImageSharp - reverting nuget from 2.1 to 1.0.4 fixed the issue immediately. So I thought you should know.
PS. Also, posting this is to help someone who's going to google for a solution desperately
Images
No response