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

[FIX#4505] FileSizeFilter make memory leak #4507

Merged
merged 9 commits into from Mar 29, 2023

Conversation

847850277
Copy link
Contributor

@847850277 847850277 commented Mar 28, 2023

Dear community
1、Corrected maxInMemorySize to - 1 to resolve theorg.springframework.core.io.buffer.DataBufferLimitException: Exceeded limit on max bytes to buffer error.
2、Finally, the DataBuffer object was released.
please review code.

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

@847850277 847850277 changed the title [Bugfix#4505] FileSizeFilter make memory leak [FIX#4505] FileSizeFilter make memory leak Mar 28, 2023
@@ -56,7 +57,7 @@ public class FileSizeFilter implements WebFilter {
private final List<HttpMessageReader<?>> messageReaders;

public FileSizeFilter(final int fileMaxSize) {
HandlerStrategies handlerStrategies = HandlerStrategies.builder().codecs(configurer -> configurer.defaultCodecs().maxInMemorySize(fileMaxSize * BYTES_PER_MB)).build();
HandlerStrategies handlerStrategies = HandlerStrategies.builder().codecs(configurer -> configurer.defaultCodecs().maxInMemorySize(-1)).build();
Copy link
Member

Choose a reason for hiding this comment

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

why modify -1 ?

}));
});
}).doOnDiscard(DataBuffer.class, DataBufferUtils::release);
Copy link
Member

Choose a reason for hiding this comment

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

doOnDiscard---》doOnFinal?

@codecov-commenter
Copy link

codecov-commenter commented Mar 29, 2023

Codecov Report

Merging #4507 (c1c660b) into master (230ed1c) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #4507      +/-   ##
============================================
+ Coverage     68.33%   68.42%   +0.09%     
- Complexity     7610     7614       +4     
============================================
  Files          1029     1029              
  Lines         29401    29403       +2     
  Branches       2659     2659              
============================================
+ Hits          20090    20118      +28     
+ Misses         7719     7694      -25     
+ Partials       1592     1591       -1     
Impacted Files Coverage Δ
...a/org/apache/shenyu/web/filter/FileSizeFilter.java 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@yu199195 yu199195 added this to the 2.6.0 milestone Mar 29, 2023
@yu199195 yu199195 added the type: bug Something isn't working label Mar 29, 2023
@yu199195 yu199195 merged commit 0a777ec into apache:master Mar 29, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants