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

Reactor2TcpClient leaks threads on shutdown [SPR-14229] #18803

Closed
spring-issuemaster opened this issue Apr 28, 2016 · 7 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Roel Rymenants opened SPR-14229 and commented

When using the StompBrokerRelayMessageHandler in a container (e.g. tomcat) it seems the shutdown sequence leaks threads.
More specifically it appears to be the reconnect functionality of the used NettyTcpClients which trigger a timer on the passed reactor.Environment, which leaks a thread per client. I'm not familiar enough with neither netty nor reactor-netty to have a further clue regarding how to proceed with solving the issue.

In order to reproduce, you can switch the spring-websocket-portfolio webapp to use a StompBrokerRelay with rabbitmq. Loading the webapp in tomcat, logging in and then unloading the app again shows the issue.
You can find a heap dump attached.


Affects: 4.2.5

Reference URL: https://github.com/rstoyanchev/spring-websocket-portfolio/

Attachments:

Issue Links:

  • #18805 Reactor2TcpClient does not remove TcpClient instances after connection closed
  • #19601 Reactor2StompTcpClient should use a shared EventLoopGroup

Referenced from: commits bd40a93, f7ace54

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Rossen Stoyanchev commented

Can you confirm if you're using Reactor? There was a change related to reconnect in 2.0.8 so worth asking.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Roel Rymenants commented

AFAIK using the broker relay requires the usage of Reactor, no?

I used the spring-websocket-project to demonstrate this issue in order for dependencies etc to be clear. The only thing changed from the master branch was switching from the SimpMessageBroker to the broker relay in the config.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Rossen Stoyanchev commented

How do you "shutdown" exactly? In the heap dump Tomcat's threads are still there.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Roel Rymenants commented

I undeploy the web application (war) through the tomcat management console

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Rossen Stoyanchev commented

I can confirm the issue. Currently we shut down the shared Netty EventLoopGroup but as you pointed out there is also a timer thread associated with the Environment.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2016

Rossen Stoyanchev commented

I've pushed a fix to shutdown the Environment which I verified locally. It will be available as 4.3.0.BUILD-SNAPSHOT or 4.2.6.BUILD-SNAPSHOT when the current build completes.

That may fix your case. Note however that there are more issues and I've created two Reactor tickets (see #623 and #624). The issue occurs especially if a reconnect is triggered while the server is still running (e.g. Rabbit goes down and then up). In any case expect a Reactor 2.0.9 release relatively soon. You can watch those tickets for when the fix becomes available to test.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 2, 2016

Roel Rymenants commented

Great! The fix does seem to address the reported problem.

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