Hi,
We are using Spring 5.3.28 in our application. This application supports dynamic plugins, so we do something like this:
var pluginJar = Paths.get("/path/to/plugin.jar");
try (var ucl = new URLClassloader(pluginJar.toURI().toURL()) { // Create a classloader to load plugin classes
Class pluginClass = ucl.findClass("com.foo.MyPluginEntryPoint"); // Use reflection to load the plugin entry point
MyPluginInterface pluginInstance = (MyPluginInterface) pluginClass.newInstance(); // MyPluginInterface is part of the API and exists in the application classloader
try (var springContext = new AnnotationConfigApplicationContext()) { // Create a dedicated Spring context for the plugin
pluginInstance.getBeans().forEach(springContext::registerBean);
springContext.refresh();
// Do something with the plugin springContext, like finding extension point by querying per interface
}
}
Files.delete(pluginJar); // Fail on Windows, file is locked
The issue I am facing is that even if I close properly the Spring context and the URLClassloader, there is still a class leak. If I run the previous code in a loop, I can see the metaspace and the number of loaded classes continuously increasing. Triggering a gc doesn't help. The URLClassloader instance (and all associated classes) is never collected.
I tried to find the reason for this, and I might be wrong at this point, but here are my findings.
The leak seems to be caused by one (or all) of the caches that are used in Spring annotation processing. My understanding is that one class inside one of our plugin we later add as a bean is annotated with a custom annotation. This annotation is part of the plugin JAR, and so belongs to the plugin classloader. But Spring will store the annotation (and transitively the classloader I'm trying to free) in one of the static top-level annotations caches.
Am I on the right track?
I managed to unload properly my plugin only after doing this:
ReflectionUtils.clearCache();
try {
var orderCacheField = OrderUtils.class.getDeclaredField("orderCache");
orderCacheField.setAccessible(true);
Map orderCahce = (Map) orderCacheField.get(OrderUtils.class);
orderCahce.clear();
Class<?> standardRepeatableContainersClass = Class.forName("org.springframework.core.annotation.RepeatableContainers$StandardRepeatableContainers");
var cacheField = standardRepeatableContainersClass.getDeclaredField("cache");
cacheField.setAccessible(true);
Map cache = (Map) cacheField.get(standardRepeatableContainersClass);
cache.clear();
Class<?> attributeMethodClass = Class.forName("org.springframework.core.annotation.AttributeMethods");
var attCacheField = attributeMethodClass.getDeclaredField("cache");
attCacheField.setAccessible(true);
Map attCache = (Map) attCacheField.get(attributeMethodClass);
attCache.clear();
Class<?> annotationTypeMappingsClass = Class.forName("org.springframework.core.annotation.AnnotationTypeMappings");
var noRepeatablesCacheField = annotationTypeMappingsClass.getDeclaredField("noRepeatablesCache");
noRepeatablesCacheField.setAccessible(true);
Map noRepeatablesCache = (Map) noRepeatablesCacheField.get(annotationTypeMappingsClass);
noRepeatablesCache.clear();
var standardRepeatablesCacheField = annotationTypeMappingsClass.getDeclaredField("standardRepeatablesCache");
standardRepeatablesCacheField.setAccessible(true);
Map standardRepeatablesCache = (Map) standardRepeatablesCacheField.get(annotationTypeMappingsClass);
standardRepeatablesCache.clear();
Class<?> annotationScannerClass = Class.forName("org.springframework.core.annotation.AnnotationsScanner");
var declaredAnnotationCacheField = annotationScannerClass.getDeclaredField("declaredAnnotationCache");
declaredAnnotationCacheField.setAccessible(true);
Map declaredAnnotationCache = (Map) declaredAnnotationCacheField.get(annotationScannerClass);
declaredAnnotationCache.clear();
} catch (Exception e) {
throw new RuntimeException(e);
}
(not sure they are all required, I made so many attempts...)
Before I move further, do you have some insights to share? Shouldn't we have a clean way to clean all those caches?
I can probably provide a reproducer, but since this will need quite some effort, I want to be sure first I did not miss something obvious.
Thanks.
Comment From: jhoeller
We use ClassUtils.isCacheSafe in several places, e.g. CachedIntrospectionResults. For custom annotations, we need to do the same for the annotation caches.
Comment From: henryju
In CachedIntrospectionResults you are using ClassUtils.isCacheSafe to decide to use either ConcurrentHashMap or ConcurrentReferenceHashMap.
ConcurrentReferenceHashMap uses soft references, that are only garbage collected when the system goes low on memory, so that would not help me to "force" the GC to collect those classes.
BTW, in all the caches I mentioned earlier, you are already using ConcurrentReferenceHashMap.
Since yesterday, I continued to learn :) "Unloading" classes in Java is a tricky topic apparently. Any Java application trying to support hot reloading is affected. I found for example some interesting code in the IntelliJ Platform, where they are trying to unload dynamic plugins classloaders. The "trick" they use is to saturate the memory to force the GC to release soft references. Another idea would be for you to use weak references in all the caches, but that would make them not very useful.
In the end, since unloading classloader is probably not a common use case, maybe that would be simpler to expose methods allowing to purging of those caches when needed, similar to ReflectionUtils.clearCache()?
Comment From: jhoeller
Good point, and such a clearCache() method already exists on AnnotationUtils, clearing most (but not all) of the internal annotation caches. I'll try to revise that towards complete clearing so that a single AnnotationUtils.clearCache() call is sufficient.
Comment From: jhoeller
This turns out to be a regression from 4.3 since two new caches introduced in 5.2 where not part of AnnotationUtils.clearCache() yet.
Comment From: jhoeller
We also reset our common infrastructure caches again in AbstractApplicationContext.close() now in order to avoid class reference leaks on shutdown, not just after refresh() where we have been doing this for memory footprint purposes already.
Comment From: henryju
Thanks, that looks great!