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

athenad: fix memory leak by closing Response objects #34101

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

deanlee
Copy link
Contributor

@deanlee deanlee commented Nov 24, 2024

Fixes a common memory leak when using the requests library, where unclosed HTTP Response objects prevent garbage collection and cause memory leaks. if Response objects are not explicitly closed, requests retains references to them, causing resource exhaustion over time.

Since we can't use a with statement due to mock_put = mocker.patch('requests.put') in test_athenad.py, this fix explicitly closes the Response object after use.This ensures proper memory management and prevents resource exhaustion, particularly during large or repeated uploads.

see psf/requests#4601 (comment) for details

resolve: #34079

Note: We should also review other areas where the requests library is used, such as the uploader, to ensure that Response objects are properly closed and to prevent potential memory leaks.

@deanlee deanlee marked this pull request as draft November 24, 2024 13:32
@deanlee deanlee force-pushed the athenad_fix_requests_memory_leak branch 2 times, most recently from 45f799b to e99fab9 Compare November 24, 2024 13:50
@deanlee deanlee marked this pull request as ready for review November 24, 2024 14:00
@sshane
Copy link
Contributor

sshane commented Nov 27, 2024

can the test be switched to a with statement? some static analysis for unclosed files/requests/contexts would be great, can you check if ruff has a rule for that?

@deanlee deanlee force-pushed the athenad_fix_requests_memory_leak branch from e99fab9 to d1f097c Compare November 27, 2024 12:39
@deanlee
Copy link
Contributor Author

deanlee commented Nov 27, 2024

checked, ruff doesn't support detecting unclosed files/requests/contexts

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.

Possible athenad memory leak
3 participants