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

Cancel replaced timeouts to avoid leak #1732

Merged
merged 1 commit into from
Aug 2, 2020

Conversation

will-lauer
Copy link
Contributor

Fix for issue #1731.

When setting the TimeoutHolder, cancel any prior timeout so that they don't leak.

Previously, they would just be lost and remain in the timer until their timeout expired, which could be a long time. Previously, encountering a redirect would trigger this code, causing the old request timer to be replaced with a new one. The old timer would maintain a link back to this Future, but couldn't be canceled by this future (as its reference had been overwritten) causing the Future, is associated Response, and any AsyncResponseHandler to be retained in memory instead of being garbage collected once the request had been processed.

Fix for issue AsyncHttpClient#1731.

When setting the TimeoutHolder, cancel any prior timeout so that they don't leak. 

Previously, they would just be lost and remain in the timer until their timeout expired, which could be a long time. Previously, encountering a redirect would trigger this code, causing the old request timer to be replaced with a new one. The old timer would maintain a link back to this Future, but couldn't be canceled by this future (as its reference had been overwritten) causing the Future, is associated Response, and any AsyncResponseHandler to be retained in memory instead of being garbage collected once the request had been processed.
@slandelle slandelle merged commit 12f4b2a into AsyncHttpClient:master Aug 2, 2020
@slandelle
Copy link
Contributor

Thanks!

@will-lauer will-lauer deleted the will-lauer-patch-1731 branch August 3, 2020 14:35
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.

2 participants