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

ConnectionProvider dispose leaves behind threads #846

Closed
rstoyanchev opened this issue Sep 13, 2019 · 1 comment
Closed

ConnectionProvider dispose leaves behind threads #846

rstoyanchev opened this issue Sep 13, 2019 · 1 comment
Labels
type/bug A general bug
Milestone

Comments

@rstoyanchev
Copy link
Contributor

The ReactorResourceFactory in the Spring Framework provides an option to use resources independent of the global ones and to clean those up when the ApplicationContext is closed. This is useful for example in a Spring MVC app using the WebClient where an app may be stopped and started many times without stopping the server. In such cases Tomcat complains about threads being started but not stopped and a memory leak.

This works for LoopResources. However ConnectionProvider#dispose causes elastic-evictor-1 and elastic-2 to be created, which is probably due to the use of Schedulers.elastic() call in PooledConnectionProvider.

Considering the goal of cleaning up resources, dispose should not be creating more resources. Either it should use a different thread that it then also shuts down. Or it should provide a disposeNow method that blocks the current thread. Most often on shutdown you need to wait so this is a useful option to have.

This was originally reported in spring-projects/spring-framework#23594 and was confirmed against Reactor Netty 0.9 RC1.

@rstoyanchev rstoyanchev added type/bug A general bug status/need-triage A new issue that still need to be evaluated as a whole labels Sep 13, 2019
@smaldini smaldini added this to the 0.9.0.RC2 milestone Sep 15, 2019
@violetagg violetagg removed the status/need-triage A new issue that still need to be evaluated as a whole label Sep 16, 2019
@bclozel
Copy link
Member

bclozel commented Sep 17, 2019

See c720de5 for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants