Skip to content

Fix #10378,ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) ignores  sampling interval#10383

Merged
normanmaurer merged 4 commits intonetty:4.1from
skyguard1:4.1
Jul 1, 2020
Merged

Fix #10378,ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) ignores  sampling interval#10383
normanmaurer merged 4 commits intonetty:4.1from
skyguard1:4.1

Conversation

@skyguard1
Copy link
Copy Markdown
Contributor

@skyguard1 skyguard1 commented Jun 30, 2020

Motivation:

As described in #10378 this issue, a simple question, please review this pr, thank you

Modification:

ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) use the second parameter as the sampling interval of the newly created ResourceLeakDetector.

Result:

Fixes #10378

If there is no issue then describe the changes introduced by this PR.

xingrufei added 2 commits June 30, 2020 19:21
@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

3 similar comments
@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

*
* @param resource the resource class used to initialize the {@link ResourceLeakDetector}
* @param <T> the type of the resource class
* @param <T> the type of the resource class
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please revert any change which is just reformatting ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok,i will revert any code format

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, forgive me for not understanding the formatting style of netty code, if there is pr in the future, I will pay attention to this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skyguard1 don't worry... thanks for the PR :)

This reverts commit 87531c7
@normanmaurer
Copy link
Copy Markdown
Member

@skyguard1 can you please sign our icla and let me know once done ?

https://netty.io/s/icla

@normanmaurer
Copy link
Copy Markdown
Member

@netty-bot test this please

*/
@SuppressWarnings("deprecation")
public <T> ResourceLeakDetector<T> newResourceLeakDetector(Class<T> resource, int samplingInterval) {
return newResourceLeakDetector(resource, ResourceLeakDetector.SAMPLING_INTERVAL, Long.MAX_VALUE);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about to add a check ObjectUtil.checkPositive(samplingInterval, "samplingInterval") ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is feasible. It is possible that this parameter is the user's configuration. Is it appropriate to make an assertion here?@normanmaurer

@skyguard1
Copy link
Copy Markdown
Contributor Author

@skyguard1 can you please sign our icla and let me know once done ?

https://netty.io/s/icla

Yes, I have finished signing my name

@normanmaurer
Copy link
Copy Markdown
Member

@netty-bot test this please

@normanmaurer normanmaurer added this to the 4.1.51.Final milestone Jul 1, 2020
@normanmaurer normanmaurer merged commit 523dc5c into netty:4.1 Jul 1, 2020
@normanmaurer
Copy link
Copy Markdown
Member

@skyguard1 thanks a lot

normanmaurer pushed a commit that referenced this pull request Jul 1, 2020
… int) ignores  sampling interval (#10383)

Motivation:

newResourceLeakDetector(...) did not correctly pass the samplingInterval parameter and so it was ignored.

Modification:

ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) use the second parameter as the sampling interval of the newly created ResourceLeakDetector.

Result:

Fixes #10378
@skyguard1
Copy link
Copy Markdown
Contributor Author

@skyguard1 thanks a lot

a piece of cake,you are really kind,appreciate it

@trustin
Copy link
Copy Markdown
Member

trustin commented Jul 1, 2020

Thanks, @skyguard1 and @normanmaurer 😄

Kvicii pushed a commit to Kvicii/netty that referenced this pull request Jul 3, 2020
* '4.1' of github.com:netty/netty:
  Correctly include TLS1.3 ciphers in the enabled ciphersuites when using BoringSSL (netty#10388)
  Fix netty#10378,ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) ignores  sampling interval (netty#10383)
ihanyong pushed a commit to ihanyong/netty that referenced this pull request Jul 31, 2020
…lass, int) ignores  sampling interval (netty#10383)

Motivation:

newResourceLeakDetector(...) did not correctly pass the samplingInterval parameter and so it was ignored.

Modification:

ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) use the second parameter as the sampling interval of the newly created ResourceLeakDetector.

Result:

Fixes netty#10378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ResourceLeakDetectorFactory.newResourceLeakDetector(Class, int) ignores sampling interval.

5 participants