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

SpringBootExceptionHandler thread local leak reported by Tomcat #40206

Closed
stefanodelfalk opened this issue Apr 8, 2024 · 4 comments
Closed
Labels
status: duplicate A duplicate of another issue

Comments

@stefanodelfalk
Copy link

To reproduce (this was on Java 17.0.10, Tomcat 10.1.19, Spring Boot 3.2.4):

  1. Define any spring.main property (e.g. spring.main.banner_mode=off).
  2. Deploy your application as a WAR file in Tomcat.
  3. Start Tomcat, then exit with ctrl-c.
  • On startup, from SpringApplication.bindToSpringApplication, the JavaBeanBinder for "spring.main" finds the property defined in hasKnownBindableProperties and does not skip binding.
  • When it gets to the "spring.main.spring-boot-exception-handler" property, it has found the get method SpringApplication.getSpringBootExceptionHandler but not yet invoked it.
  • Because we're running in Tomcat, we have the JndiPropertySource, so the test containsNoDescendantOf in Binder.bindObject returns false, causing the property to be bound.
  • Finally, there was a change in DefaultBindConstructorProvider somewhere between 2.7.18 and 3.0.13 so it now not only calls bindable.getValue() but also bindable.getValue().get(). This invokes getSpringBootExceptionHandler (on the main thread) so the ThreadLocal is always created.

Tomcat's WebappClassLoaderBase.checkThreadLocalsForLeaks will log this as a severe problem when you stop the web application.

I can't really see a good way around this.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 8, 2024
@wilkinsona
Copy link
Member

Thanks for the report, @stefanodelfalk. Are you making use of JNDI in your application? For example, are you configuring properties in JNDI and then consuming them through Spring's Environment? If not, I believe you could work around this problem by adding a spring.properties file to your app that contains spring.jndi.ignore=true. This will disable JndiPropertySource and should hopefully prevent the unwanted creation of the thread local.

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

This is another problem that could be solved by #34616.

Alternatively, a more specific fix would be to stop binding directly to SpringApplication and use a SpringApplicationProperties class instead and then map those onto the SpringApplication instance. This has the potential to be a breaking change if someone's using a custom SpringApplication sub-class and they're using custom spring.main.* properties to configure it. That feels to me to be quite an unlikely combination. If someone was affected, overriding bindToSpringApplication would allow them to fix the problem.

@philwebb
Copy link
Member

philwebb commented May 1, 2024

We've also just opened #40592. I'm going to close this one as a duplicate and either #40592 or #34616 will ultimately provide a fix.

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2024
@philwebb philwebb added status: duplicate A duplicate of another issue and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels May 1, 2024
@stefanodelfalk
Copy link
Author

Thanks a lot for looking into this, looking forward to #40592.

FYI: in the application I'm working on customers have the option define data sources in JNDI. The javadoc on JndiLocatorDelegate.IGNORE_JNDI_PROPERTY_NAME seems to indicate that this should work even with ignore=true, so that may not be a problem.

However, a bigger issue is that since the beginning of time we've asked them to set an application-critical property in Tomcat's web application , and it looks like that is ignored when I set spring.jndi.ignore=true, so the workaround wouldn't work for our particular case. (We've listed the log message as a known issue for now.)

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

4 participants