Umbraco-cms: Memory Leak in GetCacheItem cache dependency

[techcolin]

Summary

We noticed memory creep in our Umbraco instances and started investigating.
Over time our sites were filling RAM on the instance and only a restart would clear it down - often by around 50%.

Running dotMemory to analyse memory dumps pointed towards millions of duplicate strings from the Umbraco Forms module config file path. While each is tiny the volume amounts to significant RAM.

We have done some local analysis and created a test controller to see what was going on. I have attached this controller for your interest. All it does is run a number of GetCacheItem calls in the same way the Umbraco Forms module calls this to load in config settings in:

Umbraco.Forms.Core.Cofiguration.GetFormsSettingFromCache

I raised this as an Umbraco Forms bug however the developer explained their cache extensions are built on top of Umbraco cache extensions, so I investigated further.

Umbraco version

We have replicated this in both Umbraco 7.14 and also in Umbraco 8.5.1

Reproduction

MemLeakController.txt

Test 1
Run the controller method a number of times while running a memory profiler (ideally dotMemory)
TestBasicCache?useFileDependency=true&nIterations=100000

Each time the number of objects in memory will increase by roughly 100,000
Check the report and you will see these are all duplicate strings for the file dependency path (with a number appended). In this case: C:\Temp\tempyfile.config8278623472

Test 2
Restart and run the controller a number of times with useFileDependency=false and you'll see the duplicate strings are no longer created

Forcing Garbage Collection has no impact, as these are all in Unmanaged Memory.

We feel there is an issue with string interning inside the cache extensions somewhere that needs addressed.

Our dotMemory profiles for these tests on U7 and U8 are available on request, and I have attached a memory graph from the Azure app service which shows the creep with organic traffic over 5 days.

I hope you can help as we are having to restart our app pools each week as these are high volume sites.

---

_This item has been added to our backlog AB#5774_

[techcolin]

I meant to post this on Friday, but here's the memory profile we're referring to. This is test data but on our production environment we are seeing millions of duplicates for this form config path.

[TimGeyssens]

Wow thanks for the detailed investigation @techcolin we had on issue on one of our biggest sites recently and pinpointed it to Forms... hope this gets some attention

[Shazwazza]

PR is here https://github.com/umbraco/Umbraco-CMS/pull/7847 ready for review

[Shazwazza]

PR for v7 is here https://github.com/umbraco/Umbraco-CMS/pull/7848

[clausjensen]

@Shazwazza both PRs look good to me - nice find!
I've merged in both (I haven't tested but I'm confident your tests are sufficient)

[nul800sebastiaan]

Thanks for fixing @Shazwazza and many thanks for the thorough investigations @techcolin - glad I could point you in the right way and really appreciate your deep dive! ⭐️ 👍

[techcolin]

Thanks for your effort on validating and fixing this @Shazwazza
Looking forward to the release and putting this one to bed.

[TimGeyssens]

Any update when this is coming to v7?

[nul800sebastiaan]

Unfortunately we have no release date for 7.15 yet.

[TimGeyssens]

Ok, and a guesstimate? Are we speaking in week(s), month(s)?

[robertjf]

there are a couple of bug fixes in 7.15.5 that we're currently implementing with patched code - any chance we could get an idea on when this will be released? It's only been like 7+ months since the last substantial release of v7... (not counting the security patch release a month and a half ago to patch ClientDependency)

[techcolin]

I tried to download source and work from the pull request, but I can't get the solution to build due to some namespace conflicts and missing dependencies. I don't have the domain knowledge or time to get this happening so we're fully depending on you guys to release this.

A memory leak which is killing our sites surely qualifies as a critical hotfix and should be released as a priority?

[SteveVaneeckhout]

@techcolin Before you build the solution you need to run the build task from the Task Runner Explorer or run the build.ps1 script in the build folder. More info

@nul800sebastiaan Keep in mind that most Umbraco websites are still running on version 7. Considering that it's not possible to easily upgrade a website to version 8 (especially bigger websites with plugins) you shouldn't wait such a long time before releasing an update.

[nul800sebastiaan]

@SteveVaneeckhout Note that this memory leak has existed for years, so your situation hasn't suddenly changed. It also only applies if you're using Forms or have custom code using the leaky method.

We want to get v7 out as soon as we can but have no estimated date yet.

[TimGeyssens]

@nul800sebastiaan so how about a patch? That shouldn't be to much effort?

Pretty weird comment about "existed for years", do you do the same with security issues? It's like the Fire department telling, well your house was on fire an hour ago, your situation hasn't changed so have some patience, we'll get there when we get there...

[nul800sebastiaan]

We're working on getting this release ready as soon as we can, I just can not promise any dates at the moment. Our assessment at the moment is that it is high priority but not critical (the house is flammable but not on fire).

Make sure to follow https://twitter.com/umbracostatus or https://status.umbraco.io/ for updates on upcoming releases.

[Pauldubbya]

@nul800sebastiaan Thanks Seb, just seen this is going to be released next week as part of 7.15.5, I'm very happy about this, our v7 sites that had high Umb forms usage were a total pain for hogging memory on our Azure App Service plans ! Many thanks :-)