Hi guys,
I did some load tests i.e
1800 requests in
60 seconds and managed to consistently reproduce an error in the caching (only 2 or 3 of all requests fail with that error).
It is basically an error caused when several threads/requests access the
CacheKey objects that are static and shared between all threads and eventually you end up with the error below:
Collection was modified; enumeration operation may not execute.at System.ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion()
at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
at Nop.Core.Caching.MemoryCacheManager.SetupEntry(ICacheEntry entry, CacheKey key) in C:\DEV\GitHub\nopCommerce_latest\src\Libraries\Nop.Core\Caching\MemoryCacheManager.cs:line 75
at Nop.Core.Caching.MemoryCacheManager.<>c__DisplayClass7_0`1.<Get>b__0(ICacheEntry entry) in C:\DEV\GitHub\nopCommerce_latest\src\Libraries\Nop.Core\Caching\MemoryCacheManager.cs:line 58
at Microsoft.Extensions.Caching.Memory.CacheExtensions.GetOrCreate[TItem](IMemoryCache cache, Object key, Func`2 factory)You won't be able to reproduce it easily unless you make a number of simultaneous requests but the problem is a quite standard one so you don't really need to reproduce it in order to fix it.
It seems like the
foreach of the
key.Prefixes in the
SetupEntry method in
MemoryCacheManager is the problematic one since it uses enumerator and when the elements of the Prefixes collection are changed at the same time you enumerate the collection you will end up with this error.
Here is the exact line in the code:
https://github.com/nopSolutions/nopCommerce/blob/8c1e1c59aea1a5cbb8fd15d335773d2c2508fb73/src/Libraries/Nop.Core/Caching/MemoryCacheManager.cs#L69I changed the code to simply use a
for loop rather than a
foreach statement and this seems to fixes the problem. Here is the code that I used:
for (var i = 0; i < key.Prefixes.Count; i++)
{
var tokenSource = _prefixes.GetOrAdd(key.Prefixes[i], new CancellationTokenSource());
entry.AddExpirationToken(new CancellationChangeToken(tokenSource.Token));
}
I made quite a lot of load tests after the change (even some with much more requests) and I can no longer reproduce the problem so I can guarantee that this fix will work :)))
p.s: What is also bothering me is that the
CacheKey objects are
static/shared and the
FillCacheKey method is changing the elements in the
Prefixes collection that is later used in the cache manager. So it is quite possible that if
Request1 sets the prefix to
Prefix1 and then before being added to the cache (before
SetupEntry is called in the cache manager) another request i.e
Request2 sets the prefix to
Prefix2 (by using different key objects) then the cache entry for
Request1 will be added as
Prefix2 rather than as
Prefix1 in the
_prefixes collection of the
MemoryCacheManager. This could potentially cause caching issues in the store. Of course now this is hard to be reproduce by a single user playing with the store but when many users browse different pages simultaneously it is quite possible to happen.
I hope you understand me but since its too late now for me you can ask if nothing is clear :)
Thanks,
Boyko