Fix CookieJar memory leak in filter_cookies() #11054
Conversation
This PR fixes a memory leak in `CookieJar.filter_cookies()` that was causing unbounded memory growth when making requests to different URL paths. The issue was introduced in version 3.10.0 and could cause memory usage to grow from ~100MB to ~5GB over weeks of operation.
The `filter_cookies()` method was inadvertently creating empty cookie entries for every domain-path combination it checked. This happened because:
1. The method generates all possible domain-path combinations for a request URL
2. It then accesses `self._cookies[p]` for each combination
3. Since `self._cookies` is a `defaultdict(SimpleCookie)`, accessing a non-existent key creates an empty `SimpleCookie` object
4. These empty objects accumulate over time, causing the memory leak
Added a simple check before accessing the dictionary:
```python
if p not in self._cookies:
continue
```
This prevents the creation of empty cookie entries while maintaining all existing functionality.
- Added regression test `test_filter_cookies_does_not_leak_memory()` that verifies the internal storage doesn't grow when filtering cookies for different paths
- All existing cookie tests pass
- Manual testing confirms the memory leak is resolved
- Fixes memory leak issue #11052
- No API changes
- No behavioral changes for existing code
- Minimal performance impact (one additional dictionary lookup per domain-path combination)
bdraco
added
backport-3.12
backport-3.13
psf-chronographer
bot
added
the
bot:chronographer:provided
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11054 +/- ##
==========================================
+ Coverage 98.79% 98.81% +0.02%
==========================================
Files 129 129
Lines 41367 41391 +24
Branches 2226 2231 +5
==========================================
+ Hits 40869 40902 +33
+ Misses 345 339 -6
+ Partials 153 150 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #11054 will not alter performanceComparing Summary
|
|
I think _morsel_cache might be leaking as well |
|
Actually probably not since its already guarded and synchronized on _cookies |
|
Yeah its ok |
|
Once we fix the leak here it solves the cache as well |
…ion in the future
Backport to 3.11: 💚 backport PR created✅ Backport PR branch: Backported as #11067 🤖 @patchback |
(cherry picked from commit e2eb195)
Backport to 3.12: 💚 backport PR created✅ Backport PR branch: Backported as #11068 🤖 @patchback |
(cherry picked from commit e2eb195)
Backport to 3.13: 💚 backport PR created✅ Backport PR branch: Backported as #11069 🤖 @patchback |
(cherry picked from commit e2eb195)
…er_cookies() (#11069) Co-authored-by: J. Nick Koston <[email protected]> Fixes #11052 memory leak issue
…er_cookies() (#11068) Co-authored-by: J. Nick Koston <[email protected]> Fixes #11052 memory leak issue
…er_cookies() (#11067) Co-authored-by: J. Nick Koston <[email protected]> Fixes #11052 memory leak issue
2 participants
Summary
This PR fixes a memory leak in
CookieJar.filter_cookies()that was causing unbounded memory growth when making requests to different URL paths. The issue was introduced in version 3.10.0 and could cause memory usage to grow from ~100MB to ~5GB over weeks of operation.The Problem
The
filter_cookies()method was inadvertently creating empty cookie entries for every domain-path combination it checked. This happened because:self._cookies[p]for each combinationself._cookiesis adefaultdict(SimpleCookie), accessing a non-existent key creates an emptySimpleCookieobjectThe Fix
Added a simple check before accessing the dictionary:
This prevents the creation of empty cookie entries while maintaining all existing functionality.
Testing
test_filter_cookies_does_not_leak_memory()that verifies the internal storage doesn't grow when filtering cookies for different pathsImpact