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.

Sign up for GitHub

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

Jump to bottom

AccessViolationException during OilPaint #2518

Closed
4 tasks done
rreminy opened this issue Aug 24, 2023 · 5 comments
Closed
4 tasks done

AccessViolationException during OilPaint #2518

rreminy opened this issue Aug 24, 2023 · 5 comments
Labels
bug

Comments

@rreminy
Copy link

rreminy commented Aug 24, 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.1

Other ImageSharp packages and versions

None

Environment (Operating system, version and so on)

Windows 10 and Debian GNU/Linux trixie/sid

.NET Framework version

7.0

Description

I have a small program that applies effects to multiple images in parallel, and after a small change I found that my program crashes quite often.

The exception produced is (Debug mode):

Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at SixLabors.ImageSharp.Memory.MemoryGroup`1+Owned[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Dispose()
   at SixLabors.ImageSharp.Memory.Buffer2D`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Dispose()
   at SixLabors.ImageSharp.ImageFrame`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Dispose(Boolean)
   at SixLabors.ImageSharp.ImageFrame.Dispose()
   at SixLabors.ImageSharp.ImageFrameCollection`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Dispose(Boolean)
   at SixLabors.ImageSharp.ImageFrameCollection.Dispose()
   at SixLabors.ImageSharp.Image`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Dispose(Boolean)
   at SixLabors.ImageSharp.Image.Dispose()
   at OilPaintRepro.Program.AttemptRepro(Int32)
   at System.Threading.Tasks.Parallel+<>c__DisplayClass19_0`1[[System.__Canon, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].<ForWorker>b__1(System.Threading.Tasks.RangeWorker ByRef, Int32, Boolean ByRef)
   at System.Threading.Tasks.TaskReplicator+Replica.Execute()
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(System.Threading.Thread, System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(System.Threading.Tasks.Task ByRef, System.Threading.Thread)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()

Steps to Reproduce

using SixLabors.ImageSharp;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Processing;
using System.Threading.Tasks;

namespace OilPaintRepro
{
    public static class Program
    {
        public static void Main(string[] args)
        {
            Parallel.For(0, int.MaxValue, AttemptRepro);
        }

        public static void AttemptRepro(int attempt)
        {
            using var image = new Image<RgbaVector>(1920, 1200, new(127, 191, 255));
            using var oiled = image.Clone();
            image.Mutate(ctx => ctx.OilPaint());
        }
    }
}

It can takes a few seconds to a few minutes depending on your luck.

Images

No images needed
@rreminy
Copy link
Author

rreminy commented Aug 24, 2023

I realize this exception is a little different from the one I was getting, which vs2022 was pointing here:

ImageSharp/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs

Line 151 in 1920e28

Unsafe.Add(ref intensityBinRef, (uint)currentIntensity)++;

I tried a few times but I cannot get it there, however I got a different variation of the stack trace:

Fatal error. Internal CLR error. (0x80131506)
   at System.Buffers.IMemoryOwner`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].get_Memory()
   at SixLabors.ImageSharp.Processing.Processors.Effects.OilPaintingProcessor`1+RowIntervalOperation[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Invoke(SixLabors.ImageSharp.Memory.RowInterval ByRef)
   at SixLabors.ImageSharp.Advanced.ParallelRowIterator+RowIntervalOperationWrapper`1[[SixLabors.ImageSharp.Processing.Processors.Effects.OilPaintingProcessor`1+RowIntervalOperation[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Invoke(Int32)
   at System.Threading.Tasks.Parallel+<>c__DisplayClass19_0`1[[System.__Canon, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].<ForWorker>b__1(System.Threading.Tasks.RangeWorker ByRef, Int32, Boolean ByRef)
   at System.Threading.Tasks.TaskReplicator+Replica.Execute()
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(System.Threading.Tasks.Task ByRef, System.Threading.Thread)
   at System.Threading.Tasks.ThreadPoolTaskScheduler.TryExecuteTaskInline(System.Threading.Tasks.Task, Boolean)
   at System.Threading.Tasks.TaskScheduler.TryRunInline(System.Threading.Tasks.Task, Boolean)
   at System.Threading.Tasks.Task.InternalRunSynchronously(System.Threading.Tasks.TaskScheduler, Boolean)
   at System.Threading.Tasks.TaskReplicator.Run[[System.Threading.Tasks.RangeWorker, System.Threading.Tasks.Parallel, Version=7.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](ReplicatableUserAction`1<System.Threading.Tasks.RangeWorker>, System.Threading.Tasks.ParallelOptions, Boolean)
   at System.Threading.Tasks.Parallel.ForWorker[[System.__Canon, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]](Int32, Int32, System.Threading.Tasks.ParallelOptions, System.Action`1<Int32>, System.Action`2<Int32,System.Threading.Tasks.ParallelLoopState>, System.Func`4<Int32,System.Threading.Tasks.ParallelLoopState,System.__Canon,System.__Canon>, System.Func`1<System.__Canon>, System.Action`1<System.__Canon>)
   at System.Threading.Tasks.Parallel.For(Int32, Int32, System.Threading.Tasks.ParallelOptions, System.Action`1<Int32>)
   at SixLabors.ImageSharp.Advanced.ParallelRowIterator.IterateRowIntervals[[SixLabors.ImageSharp.Processing.Processors.Effects.OilPaintingProcessor`1+RowIntervalOperation[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](SixLabors.ImageSharp.Rectangle, SixLabors.ImageSharp.Advanced.ParallelExecutionSettings ByRef, RowIntervalOperation<SixLabors.ImageSharp.PixelFormats.RgbaVector> ByRef)
   at SixLabors.ImageSharp.Advanced.ParallelRowIterator.IterateRowIntervals[[SixLabors.ImageSharp.Processing.Processors.Effects.OilPaintingProcessor`1+RowIntervalOperation[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]], SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](SixLabors.ImageSharp.Configuration, SixLabors.ImageSharp.Rectangle, RowIntervalOperation<SixLabors.ImageSharp.PixelFormats.RgbaVector> ByRef)
   at SixLabors.ImageSharp.Processing.Processors.Effects.OilPaintingProcessor`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].OnFrameApply(SixLabors.ImageSharp.ImageFrame`1<SixLabors.ImageSharp.PixelFormats.RgbaVector>)
   at SixLabors.ImageSharp.Processing.Processors.ImageProcessor`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].Apply(SixLabors.ImageSharp.ImageFrame`1<SixLabors.ImageSharp.PixelFormats.RgbaVector>)
   at SixLabors.ImageSharp.Processing.Processors.ImageProcessor`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].SixLabors.ImageSharp.Processing.Processors.IImageProcessor<TPixel>.Execute()
   at SixLabors.ImageSharp.Processing.DefaultImageProcessorContext`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].ApplyProcessor(SixLabors.ImageSharp.Processing.Processors.IImageProcessor, SixLabors.ImageSharp.Rectangle)
   at SixLabors.ImageSharp.Processing.DefaultImageProcessorContext`1[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]].ApplyProcessor(SixLabors.ImageSharp.Processing.Processors.IImageProcessor)
   at SixLabors.ImageSharp.Processing.OilPaintExtensions.OilPaint(SixLabors.ImageSharp.Processing.IImageProcessingContext, Int32, Int32)
   at SixLabors.ImageSharp.Processing.OilPaintExtensions.OilPaint(SixLabors.ImageSharp.Processing.IImageProcessingContext)
   at OilPaintRepro.Program+<>c.<AttemptRepro>b__1_0(SixLabors.ImageSharp.Processing.IImageProcessingContext)
   at SixLabors.ImageSharp.Processing.ProcessingExtensions.Mutate[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](SixLabors.ImageSharp.Image`1<SixLabors.ImageSharp.PixelFormats.RgbaVector>, SixLabors.ImageSharp.Configuration, System.Action`1<SixLabors.ImageSharp.Processing.IImageProcessingContext>)
   at SixLabors.ImageSharp.Processing.ProcessingExtensions.Mutate[[SixLabors.ImageSharp.PixelFormats.RgbaVector, SixLabors.ImageSharp, Version=3.0.0.0, Culture=neutral, PublicKeyToken=d998eea7b14cab13]](SixLabors.ImageSharp.Image`1<SixLabors.ImageSharp.PixelFormats.RgbaVector>, System.Action`1<SixLabors.ImageSharp.Processing.IImageProcessingContext>)
   at OilPaintRepro.Program.AttemptRepro(Int32)
   at System.Threading.Tasks.Parallel+<>c__DisplayClass19_0`1[[System.__Canon, System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].<ForWorker>b__1(System.Threading.Tasks.RangeWorker ByRef, Int32, Boolean ByRef)
   at System.Threading.Tasks.TaskReplicator+Replica.Execute()
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(System.Threading.Thread, System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(System.Threading.Tasks.Task ByRef, System.Threading.Thread)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()

from which vs2022 is pointing me to:

ImageSharp/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs

Line 116 in 1920e28

Span<TPixel> sourceRowPixelSpan = this.source.DangerousGetRowSpan(y);

Hope this helps

@antonfirsov
Copy link
Member

antonfirsov commented Aug 25, 2023 •
edited
Loading

By refactoring Unsafe to a regular span indexer, a single basic OilPaint call will fall by IndexOutOfRangeException.

The bins buffer is only of size 4 * levels, typically 40 with the default params:

ImageSharp/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs

Line 100 in 1920e28

using IMemoryOwner<float> bins = this.configuration.MemoryAllocator.Allocate<float>(this.levels * 4);

While the currentIntensity indexer will have a value in the range of 255*3*(levels-1) (up to 2000-ish with the default params):

ImageSharp/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs

Line 149 in 1920e28

int currentIntensity = (int)MathF.Round((sourceBlue + sourceGreen + sourceRed) / 3F * (this.levels - 1));

We really should not use Unsafe in less critical processors, it's a footgun.

antonfirsov added a commit that referenced this issue Aug 25, 2023
@antonfirsov
fix #2518
6d6833e
@rreminy
Copy link
Author

rreminy commented Aug 25, 2023

I fixed the issue locally, the reason for the exception was really the values being out of range, RgbaVector values ranges from 0.0f to 1.0f. In my case some of the color values were apparently going negative. (since I changed the code from using Rgba32 to RgbaVector). I fixed this by adding a normalization step to make sure its within range.

Then I realize I made an oversight while making the repro code for us here, the values for new(127, 191, 255) are all out of range since its a new RgbaVector, not Rgba32, so it of course throws. Should had been new(0.5f, 0.75f, 1.0f) if I wanted them correct, which doesn't throw.

However seeing our work a little I agree as well with having the indexers be memory-safe, an AccessViolationException cannot be caught and will terminate the whole app, however I would had been able to catch an IndexOutOfRangeException fine, from which I have my app continue with the next task after printing the exception.

@antonfirsov
Copy link
Member

If a pixel value is out of range, we should normalize it in our processors. That said, my naiive fix attempt in #2519 seems to be wrong then.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Aug 28, 2023 •
edited
Loading

Yeah. By reverting the multiplier and using PixelConversionModifiers.Scale I can run the new test without issue. I'll also clamp levels between 1-255 (inclusive) as I don't see any clamping and see no reason to differentiate further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@JimBobSquarePants @antonfirsov @rreminy