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 issue 2485 which occur oom when using async servlet request. #3440

Merged
merged 4 commits into from
Aug 23, 2024

Conversation

Linzyoo
Copy link
Contributor

@Linzyoo Linzyoo commented Aug 19, 2024

Describe what this PR does / why we need it

There is an issue about it.

Does this pull request fix one issue?

Fixes #2485

Describe how you did it

Extends HandlerInterceptor with a callback method invoked after the start of asynchronous request handling.
When a handler starts an asynchronous request, the DispatcherServlet exits without invoking postHandle and afterCompletion. Then it will not exit the context and clean the thread-local variables for the request. So I extends AsynHandlerInterceptor which is to deal with this situation correctly.

Describe how to verify it

Debug to check context and ctentry.
Add a unit test for Async servlet request, and check the context after response.

Special notes for reviews

@CLAassistant
Copy link

CLAassistant commented Aug 19, 2024

CLA assistant check
All committers have signed the CLA.

# Conflicts:
#	sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/AbstractSentinelInterceptor.java
#	sentinel-demo/sentinel-demo-spring-webmvc/src/main/java/com/alibaba/csp/sentinel/demo/spring/webmvc/config/InterceptorConfig.java
2. improve based on review comments
@Linzyoo
Copy link
Contributor Author

Linzyoo commented Aug 22, 2024

finished imporving codes based on the review comments. And update the webmvc-v6x too which has the same issue.

Copy link
Collaborator

@LearningGp LearningGp left a comment

Choose a reason for hiding this comment

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

LGTM

@LearningGp LearningGp added area/integrations Issues or PRs related to integrations with open-source components kind/bug Category issues or prs related to bug. labels Aug 23, 2024
@LearningGp LearningGp merged commit 195150b into alibaba:1.8 Aug 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/integrations Issues or PRs related to integrations with open-source components kind/bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OOM occured with StringResourceWrapper linked CtEntry in Tomcat TaskThread ThreadLocal.
3 participants