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

Cache image reponses are not disposed #162

Closed
4 tasks done
deanmarcussen opened this issue Apr 29, 2021 · 3 comments
Closed
4 tasks done

Cache image reponses are not disposed #162

deanmarcussen opened this issue Apr 29, 2021 · 3 comments
Labels

Comments

@deanmarcussen
Copy link
Collaborator

deanmarcussen commented Apr 29, 2021

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp.Web
  • 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

Description

There is a couple of reported memory related issues, and while I know there is some tracking in ImageSharp itself about releasing retained resources, there looks to be another issue here (easily resolved) regarding stream usage.

Reported here on Orchard Core OrchardCMS/OrchardCore#9060
and likely related to this discussion #161

Performance testing locally this causes memory usage to grow dramatically - got as high as 15Gb before I stopped it.

Collecting manually doesn't drop it, so I suspect that's why there's reported app service failures.

Steps to Reproduce

This line doesn't dispose of the cache file streams

await imageContext.SendAsync(stream ?? await cacheResolver.OpenReadAsync(), metadata);

adding a dispose

                    using (var stream = await cacheResolver.OpenReadAsync())
                    {
                        await imageContext.SendAsync(stream, metadata);
                    }

and everything gets collected when the GC runs, and memory usage is as one would expect.

I've seen some (difficult to reproduce) issues with file locks on the cache images, but I am hoping this might explain those as well.

Off for the covid jab in a minute, so pr to come

System Configuration

Just locally.

@deanmarcussen
Copy link
Collaborator Author

Note: that sample code doesn't match well as I'm working of this pr at the moment for testing #160

@JimBobSquarePants
Copy link
Member

Great catch! I have time so will PR right now.

@JimBobSquarePants
Copy link
Member

Fixed by #160

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

Successfully merging a pull request may close this issue.

2 participants