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

Inconsistent use of cleaned URLs in PathMatchingResourcePatternResolver #32828

Closed
jahner2016 opened this issue May 7, 2024 · 10 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: feedback-provided Feedback has been provided type: regression A bug that is also a regression
Milestone

Comments

@jahner2016
Copy link

We have written our own plugin framework for Spring Boot where plugins (additional jar files) are loaded in their own PluginApplicationContext (derived from GenericApplicationContext) with their own PluginClassLoader (derived from URLClassLoader). Our plugin framework supports dynamic loading and unloading of plugins.

When a plugin is loadded, the jar file is copied with a unique temporary name to our work directory before loading the it with our PluginCalssLoader and creating the PluginApplicationContext.

When a plugin is unloaded, the PluginApplicationContext is closed, the PluginClassLoader is closed and the temporary jar file is deleted.

Everything works fine with Spring Boot 3.1 up to Spring Boot 3.1.11. But when we switch to Spring Boot 3.2 (even Spring Boot 3.2.5), the unloading does now work properly. The temporary jar file cannot be deleted anymore because the classloader is not freed.

What can be the reason for this issue? What has changed between the releases?

Kind regards
Jörg

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 7, 2024
@wilkinsona
Copy link
Member

This may be due to https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-3.2-Release-Notes#nested-jar-support but that's only a guess. To be more specific, we'd need to know much more about what you're doing, how your running the application, and so on.

If you would like us to spend some more time investigating, please spend some time providing a complete yet minimal sample that reproduces the problem. You can share it with us by pushing it to a separate repository on GitHub or by zipping it up and attaching it to this issue. as we don't know enough about what you're doing and how you're running the Spring Boot application.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label May 7, 2024
@jahner2016
Copy link
Author

jahner2016 commented May 10, 2024

Hi Andy,

I've put together a small sample which demonstrates the issue (attached zip). I've reduced our plugin framework to only the relevant parts and removed everything else like dependency management, resource loading and everything else. While assembling the demo, I've seen that there must have been something changed in the request mapping stuff of the Spring Framework. As long as I do not add @controller classes to my plugins, they are unloaded sucessfully in Spring Boot 3.2. If I add @controller classes to my plugins, they are unloaded successfully in Spring Boot 3.1.11 but not anymore in Spring Boot 3.2.5.

If you unzip the enclosed zip, you have 5 projects and a runtime folder:

  1. demo: the main Spring Boot application, containing the plugin framework implementation
  2. demo-layout: the custom layout we use for packaging plugin jars
  3. demo-plugin: the parent BOM which is used by all plugin projects
  4. demo-plugin1: a simple demo plugin with one simple controller
  5. demo-plugin2: a simple demo plugin with one simple controller
  6. runtime: the runtime folder which contains everything to start the demo

After you have extracted the zip, please execute the build-all.bat in the root folder. It builds everything and copies the targets to the correct folders. The plugin jars are copied to the plugins folder, everything else to the lib folder. Afterwards please execute the startServer.bat in runtime\bin. It should start the small web application and you should be able to see that two plugins are activated when you open "localhost:8081/list-plugins" in a browser. When you now enter the url "localhost:8081/deactivate-plugin?pluginId=demo-plugin1" you should see a line "Deleted file ..\work\demo-plugin1-1.0.0.0-.jar. That means the plugin class loader is completely unloaded and the jar could be deleted (the expected behaviour).

Now please change the Spring Boot version to 3.2.5 in demo\pom.xml, demo-layout\pom.xml and demo-plugin\pom.xml and recompile and restart everything. If you again deactivate the plugin, you will see that the output is now "Unable to delete... " allthough nothing has changed in our code.

So the problem must be in the Spring Framework and I need your help if I have to change something in our code.

Kind regards
Jörg
test-project.zip

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 10, 2024
@jahner2016 jahner2016 changed the title What has exactly changed between Spring Boot 3.1 and 3.1 according to custom class loaders What has exactly changed between Spring Boot 3.1 and 3.2 according to custom class loaders May 13, 2024
@jahner2016
Copy link
Author

jahner2016 commented May 15, 2024 via email

@wilkinsona
Copy link
Member

Thanks for the sample. I think I've managed to reproduce the behavior that you have described with a file handle being leaked but it is hard to be certain as the sample is Windows-specific and I use macOS for my day-to-day work. macOS (like Linux) also doesn't prevent a file from being deleted when it's open but I can see the leaked file handle using lsof.

The underlying cause of the problem is two URLs that are different but point to the same resource. They look something like this:

  • file:/Users/awilkinson/Downloads/test-project/runtime/bin/../work/demo-plugin1-1.0.0.0-3486320809144453673.jar
  • file:/Users/awilkinson/Downloads/test-project/runtime/work/demo-plugin1-1.0.0.0-3486320809144453673.jar

This difference results in the jar being opened twice but only closed once. As result there are two open file handles for the plugin jar when it is activated and one remains once it has been deactivated.

I first suspected that this was due to the new nested jar support but switching to the CLASSIC loader does not help. In fact, it makes things worse as there are three open file handles once the plugin has been activated and two remain once it has been deactivated. I also tried running DemoApplication in my IDE so that a Spring Boot's nested jar supported isn't used and the problem still occurs.

Given that the problem occurs without Spring Boot's nested jar support and only occurs when a component is found by classpath scanning of the plugin, I next suspected it was due to a change in Spring Framework and this appears to be the case. With the demo project updated to set the spring-framework.version property to 6.0.19 and rebuilt, the problem no longer occurs. Note that this downgrade requires running the app with -Dspring.autoconfigure.exclude=org.springframework.boot.autoconfigure.task.TaskSchedulingAutoConfiguration as the task scheduling auto-configuration requires Spring Framework 6.2.x. We'll transfer this issue to the Framework team so that they can investigate.

In the meantime, the problem can be worked around by using a canonical directory for the workDir in PluginLoader:

File workDir = new File(PluginConstants.WORK_DIR).getCanonicalFile();

This removes the bin/../ from the first URL so it's then matched by the second.

@bclozel bclozel transferred this issue from spring-projects/spring-boot May 15, 2024
@bclozel bclozel added the in: core Issues in core modules (aop, beans, core, context, expression) label May 15, 2024
@wilkinsona
Copy link
Member

wilkinsona commented May 15, 2024

Some notes for the Framework team that may help. The TL;DR is that I think that 9342317 is the cause of the regression as it calls StringUtils.cleanPath which 6.0 does not do.

Other observations that led me to this conclusion follow:

The two different URLs are used in close proximity to each other within scanCandidateComponents. When the first with the bin/../ is used, the stack is as follows:

UrlJarFiles.getOrCreate(boolean, URL) line: 72	
JarUrlConnection.connect() line: 289	
JarUrlConnection.getJarFile() line: 99	
PathMatchingResourcePatternResolver.doFindPathMatchingJarResources(Resource, URL, String) line: 683	
PathMatchingResourcePatternResolver.findPathMatchingResources(String) line: 586	
PathMatchingResourcePatternResolver.getResources(String) line: 334	
PluginApplicationContext(AbstractApplicationContext).getResources(String) line: 1511	
PluginApplicationContext(GenericApplicationContext).getResources(String) line: 262	
ClassPathBeanDefinitionScanner(ClassPathScanningCandidateComponentProvider).scanCandidateComponents(String) line: 457	
ClassPathBeanDefinitionScanner(ClassPathScanningCandidateComponentProvider).findCandidateComponents(String) line: 351	
ClassPathBeanDefinitionScanner.doScan(String...) line: 277	
ClassPathBeanDefinitionScanner.scan(String...) line: 255	
PluginApplicationContext.scan(String...) line: 44	
StandardPluginRegistry.doLoadPlugin(StandardPlugin) line: 243	
StandardPluginRegistry.loadPlugin(StandardPlugin) line: 229	
StandardPluginRegistry.loadPlugins() line: 206	
StandardPluginRegistry.afterSingletonsInstantiated() line: 156	
DefaultListableBeanFactory.preInstantiateSingletons() line: 986	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).finishBeanFactoryInitialization(ConfigurableListableBeanFactory) line: 962	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).refresh() line: 624	
AnnotationConfigServletWebServerApplicationContext(ServletWebServerApplicationContext).refresh() line: 146	
SpringApplication.refresh(ConfigurableApplicationContext) line: 754	
SpringApplication.refreshContext(ConfigurableApplicationContext) line: 456	
SpringApplication.run(String...) line: 334	
SpringApplication.run(Class<?>[], String[]) line: 1354	
SpringApplication.run(Class<?>, String...) line: 1343	
DemoApplication.main(String[]) line: 49	

When the second URL without the bin/../ is used, the stack is as follows:

UrlJarFiles.getOrCreate(boolean, URL) line: 72	
JarUrlConnection.connect() line: 289	
JarUrlConnection.getInputStream() line: 195	
UrlResource.getInputStream() line: 232		
SimpleMetadataReader.getClassReader(Resource) line: 54	
SimpleMetadataReader.<init>(Resource, ClassLoader) line: 48	
CachingMetadataReaderFactory(SimpleMetadataReaderFactory).getMetadataReader(Resource) line: 103	
CachingMetadataReaderFactory.getMetadataReader(Resource) line: 122	
ClassPathBeanDefinitionScanner(ClassPathScanningCandidateComponentProvider).scanCandidateComponents(String) line: 470	
ClassPathBeanDefinitionScanner(ClassPathScanningCandidateComponentProvider).findCandidateComponents(String) line: 351	
ClassPathBeanDefinitionScanner.doScan(String...) line: 277	
ClassPathBeanDefinitionScanner.scan(String...) line: 255	
PluginApplicationContext.scan(String...) line: 44	
StandardPluginRegistry.doLoadPlugin(StandardPlugin) line: 243	
StandardPluginRegistry.loadPlugin(StandardPlugin) line: 229	
StandardPluginRegistry.loadPlugins() line: 206	
StandardPluginRegistry.afterSingletonsInstantiated() line: 156	
DefaultListableBeanFactory.preInstantiateSingletons() line: 986	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).finishBeanFactoryInitialization(ConfigurableListableBeanFactory) line: 962	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).refresh() line: 624	
AnnotationConfigServletWebServerApplicationContext(ServletWebServerApplicationContext).refresh() line: 146	
SpringApplication.refresh(ConfigurableApplicationContext) line: 754	
SpringApplication.refreshContext(ConfigurableApplicationContext) line: 456	
SpringApplication.run(String...) line: 334	
SpringApplication.run(Class<?>[], String[]) line: 1354	
SpringApplication.run(Class<?>, String...) line: 1343	
DemoApplication.main(String[]) line: 49	

A URL in the second form can be seen in trace-level logging from ClassPathBeanDefinitionScanner:

2024-05-15T11:16:48.334+01:00 TRACE 33513 --- [           main] o.s.c.a.ClassPathBeanDefinitionScanner   : Scanning URL [jar:file:/Users/awilkinson/Downloads/test-project/runtime/work/demo-plugin1-1.0.0.0-16251652149578762890.jar!/com/example/demo/plugin1/DemoController.class]

Upon downgrading to Framework 6.0.19, this logging changes and the URL's in the first form with the bin/../:

2024-05-15T11:18:15.196+01:00 TRACE 33560 --- [           main] o.s.c.a.ClassPathBeanDefinitionScanner   : Scanning URL [jar:file:/Users/awilkinson/Downloads/test-project/runtime/bin/../work/demo-plugin1-1.0.0.0-14759968729258928182.jar!/com/example/demo/plugin1/DemoController.class]

6.1 is cleaning the path when going from a URL for a root dir resource to a URL for a specific resource that matches the sub-pattern. createRelative is called on a UrlResource with the URL jar:file:/Users/awilkinson/Downloads/test-project/runtime/bin/../work/demo-plugin1-1.0.0.0-12436866063543419511.jar!/com/example/demo/plugin1/ with a relativePath of DemoController.class. This results in a UrlResource with the URL jar:file:/Users/awilkinson/Downloads/test-project/runtime/work/demo-plugin1-1.0.0.0-12436866063543419511.jar!/com/example/demo/plugin1/DemoController.class due to path cleaning that's now performed in ResourceUtils.toURL(String).

@jhoeller jhoeller added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 15, 2024
@jhoeller jhoeller self-assigned this May 15, 2024
@jhoeller jhoeller added this to the 6.1.7 milestone May 15, 2024
@jhoeller
Copy link
Contributor

It looks like ClassLoader.getResources itself returns a uncleaned path with a ../ segment there, and our convertClassLoaderURL method turns it into a UrlResource(URL) due to a jar location - whereas it would turn it into a FileSystemResource with a clean path in case of a file location. So we should consistently use cleaned URL paths even for the jar location case there, I suppose.

@jhoeller jhoeller changed the title What has exactly changed between Spring Boot 3.1 and 3.2 according to custom class loaders Inconsistent use of cleaned URLs in PathMatchingResourcePatternResolver May 15, 2024
@jhoeller
Copy link
Contributor

This should be resolved based on my understanding above, consistently cleaning URLs from the ClassLoader.
@wilkinsona please give the upcoming 6.1.7 snapshot a try with Boot in such a scenario!

@wilkinsona
Copy link
Member

wilkinsona commented May 15, 2024

Unfortunately, the sample is broken as before when using 6.1.7-SNAPSHOT although the exact behavior has changed. Now, the first URL that's used does not contain bin/../. The stack at this point is as follows:

UrlJarFiles.getOrCreate(boolean, URL) line: 72	
JarUrlConnection.connect() line: 289	
JarUrlConnection.getJarFile() line: 99	
PathMatchingResourcePatternResolver.doFindPathMatchingJarResources(Resource, URL, String) line: 694	
PathMatchingResourcePatternResolver.findPathMatchingResources(String) line: 597	
PathMatchingResourcePatternResolver.getResources(String) line: 334	
PluginApplicationContext(AbstractApplicationContext).getResources(String) line: 1511	
PluginApplicationContext(GenericApplicationContext).getResources(String) line: 263	
ClassPathBeanDefinitionScanner(ClassPathScanningCandidateComponentProvider).scanCandidateComponents(String) line: 457	
ClassPathBeanDefinitionScanner(ClassPathScanningCandidateComponentProvider).findCandidateComponents(String) line: 351	
ClassPathBeanDefinitionScanner.doScan(String...) line: 277	
ClassPathBeanDefinitionScanner.scan(String...) line: 255	
PluginApplicationContext.scan(String...) line: 44	
StandardPluginRegistry.doLoadPlugin(StandardPlugin) line: 243	
StandardPluginRegistry.loadPlugin(StandardPlugin) line: 229	
StandardPluginRegistry.loadPlugins() line: 206	
StandardPluginRegistry.afterSingletonsInstantiated() line: 156	
DefaultListableBeanFactory.preInstantiateSingletons() line: 986	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).finishBeanFactoryInitialization(ConfigurableListableBeanFactory) line: 962	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).refresh() line: 624	
AnnotationConfigServletWebServerApplicationContext(ServletWebServerApplicationContext).refresh() line: 146	
SpringApplication.refresh(ConfigurableApplicationContext) line: 754	
SpringApplication.refreshContext(ConfigurableApplicationContext) line: 456	
SpringApplication.run(String...) line: 335	
SpringApplication.run(Class<?>[], String[]) line: 1363	
SpringApplication.run(Class<?>, String...) line: 1352	
DemoApplication.main(String[]) line: 49	

The second URL that's used now does contain bin/../. The stack at this point is as follows:

UrlJarFiles.getOrCreate(boolean, URL) line: 72	
JarUrlConnection.connect() line: 289	
JarUrlConnection.getInputStream() line: 195	
PluginClassLoader(URLClassLoader).getResourceAsStream(String) line: 296	
ClassPathResource.getInputStream() line: 209	
SimpleMetadataReader.getClassReader(Resource) line: 54	
SimpleMetadataReader.<init>(Resource, ClassLoader) line: 48	
CachingMetadataReaderFactory(SimpleMetadataReaderFactory).getMetadataReader(Resource) line: 103	
CachingMetadataReaderFactory.getMetadataReader(Resource) line: 122	
CachingMetadataReaderFactory(SimpleMetadataReaderFactory).getMetadataReader(String) line: 81	
ConfigurationClassParser.asSourceClass(String, Predicate<String>) line: 630	
ConfigurationClassParser.asSourceClass(ConfigurationClass, Predicate<String>) line: 579	
ConfigurationClassParser.processConfigurationClass(ConfigurationClass, Predicate<String>) line: 244	
ConfigurationClassParser.parse(AnnotationMetadata, String) line: 197	
ConfigurationClassParser.parse(Set<BeanDefinitionHolder>) line: 165	
ConfigurationClassPostProcessor.processConfigBeanDefinitions(BeanDefinitionRegistry) line: 417	
ConfigurationClassPostProcessor.postProcessBeanDefinitionRegistry(BeanDefinitionRegistry) line: 290	
PostProcessorRegistrationDelegate.invokeBeanDefinitionRegistryPostProcessors(Collection<BeanDefinitionRegistryPostProcessor>, BeanDefinitionRegistry, ApplicationStartup) line: 349	
PostProcessorRegistrationDelegate.invokeBeanFactoryPostProcessors(ConfigurableListableBeanFactory, List<BeanFactoryPostProcessor>) line: 118	
PluginApplicationContext(AbstractApplicationContext).invokeBeanFactoryPostProcessors(ConfigurableListableBeanFactory) line: 788	
PluginApplicationContext(AbstractApplicationContext).refresh() line: 606	
StandardPluginRegistry.doLoadPlugin(StandardPlugin) line: 246	
StandardPluginRegistry.loadPlugin(StandardPlugin) line: 229	
StandardPluginRegistry.loadPlugins() line: 206	
StandardPluginRegistry.afterSingletonsInstantiated() line: 156	
DefaultListableBeanFactory.preInstantiateSingletons() line: 986	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).finishBeanFactoryInitialization(ConfigurableListableBeanFactory) line: 962	
AnnotationConfigServletWebServerApplicationContext(AbstractApplicationContext).refresh() line: 624	
AnnotationConfigServletWebServerApplicationContext(ServletWebServerApplicationContext).refresh() line: 146	
SpringApplication.refresh(ConfigurableApplicationContext) line: 754	
SpringApplication.refreshContext(ConfigurableApplicationContext) line: 456	
SpringApplication.run(String...) line: 335	
SpringApplication.run(Class<?>[], String[]) line: 1363	
SpringApplication.run(Class<?>, String...) line: 1352	
DemoApplication.main(String[]) line: 49	

An interesting change here is that, in the second use of the URL, it's now coming from a ClassPathResource. With 6.1.6, the second use of the URL was coming from a UrlResource.

I think this explains why the sample continues to be broken as the class loader has the URL file:/Users/awilkinson/Downloads/test-project/runtime/bin/../work/demo-plugin1-1.0.0.0-12470862329384363385.jar on its path. The work around that I described above (using the canonical path when working with the files that populate the plugin's classpath) continues to work as both sides are then using the cleaned path.

@jhoeller
Copy link
Contributor

jhoeller commented May 15, 2024

Thanks for the detailed analysis, @wilkinsona! Unfortunately it gets really involved from here since ClassPathResource delegates to ClassLoader.getResourceAsStream(String) which internally resolves a URL that it then obtains the stream for. We don't control those internal URLs at all unless we change the access path to ClassLoader.getResource(String), cleaning the returned URL and then manually opening a stream for it. Since that might bypass optimizations in custom ClassLoader implementations, I'm not inclined to go there.

So for the time being, PathMatchingResourcePatternResolver exposes a consistent set of URLs in its results which is a sensible measure in general. Any subsequent direct class path access, be it from ClassPathResource or through direct ClassLoader usage, will still internally use the original URL though. From that perspective, it seems necessary to enforce clean URLs in the PluginClassLoader itself if it expects to have the same resource accessed in a uniform way (in order to be able to release each resource in a consistent fashion).

@jahner2016
Copy link
Author

I really appreciate your help on this issue. I've changed our PluginClassLoader to use the getCanonicalFile() method and everything works fine now, even in our full blown plugin framework which supports a lot more than the simple demo project.

Thank you again for your help. We can now finally switch to the latest Spring Boot version and no longer have to worry about the end of the 3.1 version.

Kind regards
Jörg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: feedback-provided Feedback has been provided type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants