Fixed memory leak due to infinite static caches #2438
Conversation
Use MemoryCache with fixed SlidingExpiration. Increasing the required framework version to 4.5.1 mbdavid#2421
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition="'$(TargetFramework)' == 'netstandard1.3'"> | ||
| <PackageReference Include="System.Reflection.TypeExtensions" Version="4.5.1" /> | ||
| <PackageReference Include="System.Security.Cryptography.Algorithms" Version="4.3.1" /> | ||
| <PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="1.1.2" /> |
There was a problem hiding this comment.
I am not a maintainer of this lib, but I doubt adding Microsoft.Extensions.Caching.Memory dependency would be approved. Implementing cache expiration logic relying on what is available in BCL may be needed.
Also, since this is a general purpose library, few concerns regarding sliding cache expiration approach:
- It introduces implicit dependency on static DateTime.Now or similar, which may become a problem for unit testing (not only LiteDB itself, but any other projects that uses LiteDB in tests). Microsoft.Extensions.Caching.Memory has an ISystemClock abstraction. If LiteDB library would rely on system time for caching logic, there should be a way to supply similar abstraction to LiteDB, so implementation could be substituted if needed.
- The hard decision to have sliding expiration exactly to 1 minute may need to be parametrized.
There was a problem hiding this comment.
Thank you for your review.
"dependency would be approved" - Absolutely agree. I'll try to rewrite this when I have some free time.
"which may become a problem for unit testing" - I absolutely agree, but I still have no idea how to fix this without seriously reworking the project architecture. Static methods and static caches are not the best solution.
"need to be parametrized" - Thanks, done. At the same time, now by default the parameter does not change the existing logic - indefinite cache
Use MemoryCache with fixed SlidingExpiration.
Increasing the required framework version to 4.5.1
Not the best solution, but better than endlessly eating up memory
#2421