Imagesharp: Missing ToList in CloningImageProcessor?

[silmon27]

Prerequisites

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

Description

Shouldn't there be a ToList on line 177 of CloningImageProcessor{TPixel}.cs?

// We will always be creating the clone even for mutate because we may need to resize the canvas
IEnumerable<ImageFrame<TPixel>> frames = source.Frames.Select<ImageFrame<TPixel>, ImageFrame<TPixel>>(
  x => new ImageFrame<TPixel>(
    this.Configuration,
    targetSize.Width,
    targetSize.Height,
    x.Metadata.DeepClone()));

The resulting IEnumerable is evaluated 3 times and 3 clones get created, but only 1 is properly disposed (and returned to the ArrayPool).
This results in very weird memory behavior where over time the ArrayPool essentially gets completely bypassed and LOH objects get created on top of the ArrayPool.
When I add ToList, memory behavior is stable and (almost) no Gen 2 collections occur anymore.

Tested on Windows with latest version. And for some reason more problematic in Linux containers (GC behaves differently on that platform?).

System Configuration

• ImageSharp version: current master branch
• Environment: Windows 10 1909, Linux (Containerized)
• .NET Framework version: Core 3.1

[JimBobSquarePants]

Thanks for raising this @silmon27

Can you do two things for me please?

1. Fill in the issue template we provide so that we can track versions/os etc
2. Provide a little more detail in the form of any graphs, reports related to the memory issue

I'm really keen to investigate this issue so that would be really helpful

[silmon27]

Sorry, I don't have any graphs or anything of the kind.
I figured the above out by logging all allocations and deallocations (MemoryAllocator) and a found mismatch of 2 after resizing a jpeg image. I think the cause of this should be easy to spot if you take a look at CloningImageProcessor{TPixel}.cs#L177. You pass the IEnumerable to new Image<TPixel> before converting it to a list. Therefore the selector of that Select statement is invoked on every enumeration, thus more than one ImageFrame<TPixel> is instantiated, resulting in a leak.
Hope this helps.

[JimBobSquarePants]

Thanks @silmon27 that's enough to go by. I've just pushed a PR #1074 to fix this.

[silmon27]

Even better. Thanks.