Skip to content

Configure memory leak detection for Netty #14338

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

Closed
bclozel opened this issue Sep 6, 2018 · 6 comments
Closed

Configure memory leak detection for Netty #14338

bclozel opened this issue Sep 6, 2018 · 6 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Sep 6, 2018

We should add a property for configuring the level of leak detection with Netty.

This can be done statically, has obviously a performance cost, but is really useful if an application is dealing with DataBuffer instances or extensive customizations on the Netty engine.

This applies to both server and client use cases, and Netty offers an API for that:

ResourceLeakDetector.setLevel(Level.PARANOID);

We could also consider enabling this by default when the devtools are activated.

@bclozel bclozel added the type: enhancement A general enhancement label Sep 6, 2018
@bclozel bclozel added this to the 2.2.x milestone Sep 6, 2018
@bclozel bclozel self-assigned this Sep 6, 2018
@philwebb philwebb modified the milestones: 2.2.x, 2.x Sep 7, 2018
@bclozel
Copy link
Member Author

bclozel commented Oct 26, 2018

There are a few things going on in Framework about DataBuffer resource leaks (issues being fixed, documentation, and more); I believe this issue could help in that area.

The problem is this property translates into a static call that applies to "everything Netty": Reactor Netty client and server, drivers using Netty like Lettuce, etc - unless those libraries are repackaging Netty, like Couchbase and Neo4J).

For us the natural place to put it would be under spring.netty.leak-detection.level.
Developers can still call java -Dio.netty.leakDetection.level=advanced or the static class themselves outside of Spring Boot's control. Right now the closest configuration property we've got is spring.reactor.core.stacktrace-mode.enabled (also static call, applies to a whole lot of libraries using reactor).

@bclozel bclozel added the for: team-attention An issue we'd like other members of the team to review label Oct 26, 2018
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Oct 31, 2018
@bclozel bclozel removed this from the 2.x milestone Aug 6, 2020
@bclozel bclozel added the status: declined A suggestion or change that we don't feel we should currently apply label Aug 6, 2020
@bclozel
Copy link
Member Author

bclozel commented Aug 6, 2020

Closing as there isn't much demand for this and the JVM argument might still be the best way to achieve that.

@bclozel bclozel closed this as completed Aug 6, 2020
@bclozel bclozel removed their assignment Aug 6, 2020
@JamesChenX
Copy link

JamesChenX commented Nov 8, 2020

@bclozel Is it possible to reopen this ticket? Our project wants to set the level to ADVANCED in the development environment and SIMPLE in the production environment according to the profile in application.yaml.
(We have implemented the feature in our own project but I hope Spring can do this for developers)

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Nov 21, 2020
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Nov 30, 2020
@bclozel
Copy link
Member Author

bclozel commented Nov 30, 2020

@JamesChenX Thanks for the comment, we'll reconsider this. This is indeed a good use case.

@violetagg I'm wondering if this is enough, or if we should make this feature more general. When dealing with memory leak issues with Netty, are there other options to enable? Typically, when working on a memory leak report - what would you ask developers to enable in their applications to collect useful information? Are specific logs enabled?

I'm trying to figure out if changing the Netty flag is enough or if we can do something more here. Thanks!

@bclozel bclozel reopened this Nov 30, 2020
@bclozel bclozel removed the status: declined A suggestion or change that we don't feel we should currently apply label Nov 30, 2020
@bclozel bclozel added this to the 2.4.x milestone Nov 30, 2020
@wilkinsona
Copy link
Member

Should this be assigned to 2.5.x rather than 2.4.x? If is is an enhancement then I don't think it belongs in 2.4.x.

@bclozel bclozel modified the milestones: 2.4.x, 2.5.x Dec 7, 2020
@bclozel
Copy link
Member Author

bclozel commented Dec 7, 2020

Yes, wrong milestone indeed!

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

No branches or pull requests

4 participants