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

CachingMetadataReaderFactory does not release shared resource cache after context refresh [SPR-17527] #22059

Closed
spring-issuemaster opened this Issue Nov 21, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Nov 21, 2018

Philippe Julien opened SPR-17527 and commented

org.springframework.core.type.classreading.CachingMetadataReaderFactory.clearCache() was modified in Spring 5 to only clear the metaDataReaderCache of the LocalResourceCache instance type. When created with the constructor that takes a ResourceLoader as an argument, CachingMetadataReaderFactory will keep a strong reference on the Map that it got from the DefaultResourceLoader. This would be fine if DefaultResourceLoader.clearResourceCaches(), that is called after a context refresh, cleared the resourceCaches and the inner map that it contains, but this is not the case. So CachingMetadataReaderFactory.metaDataReaderCache remain in memory after context refresh.

In our application this increased the memory footprint by about 100mb after we updated to Spring 5.

A possible fix would be for CachingMetadataReaderFactory.clearCache() to set its metadataReaderCache to null if it's not an instance of LocalResourceCache.


Affects: 5.0.10, 5.1.2

Issue Links:

  • #22058 CachingMetadataReaderFactory metadataReaderCache isn't cleaned up after context refresh when using the resource loader cache ("is duplicated by")

Referenced from: commits 23d1049, 262c702

Backported to: 5.0.11

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 21, 2018

Juergen Hoeller commented

Well spotted! While DefaultResourceLoader.clearResourceCaches() could clear all the Map values individually, it seems sensible to let go of the shared cache reference in CachingMetadataReaderFactory.clearCache(). I'm considering to reset it to an empty local cache instead of null at that point, still being able to perform local caching in case of further use of the CachingMetadataReaderFactory instance.
,

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 21, 2018

Philippe Julien commented

While you are looking at CachingMetadataReaderFactory. It seems odd that the constructor of LocalResourceCache isn't setting the object's cacheLimit field. I can open a new issue about this if you prefer.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 22, 2018

Juergen Hoeller commented

Indeed, that constructor needs to set the cacheLimit field as well. I've addressed both issues in one pass for 5.1.3 and will also backport the changes to 5.0.11 ASAP. Thanks for raising this stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.