Skip to content

Conversation

patchback[bot]
Copy link
Contributor

@patchback patchback bot commented May 28, 2025

This is a backport of PR #11054 as merged into master (e2eb195).

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:

  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

The Fix

Added a simple check before accessing the dictionary:

if p not in self._cookies:
    continue

This prevents the creation of empty cookie entries while maintaining all existing functionality.

Testing

  • 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

Impact

Copy link

codspeed-hq bot commented May 28, 2025

CodSpeed Performance Report

Merging #11068 will not alter performance

Comparing patchback/backports/3.12/e2eb1959237c8bc46a3f0e79a77c2086145d98cf/pr-11054 (1c0d613) with 3.12 (872cab6)

Summary

✅ 59 untouched benchmarks

@bdraco bdraco enabled auto-merge (squash) May 28, 2025 21:04
Copy link

codecov bot commented May 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.24%. Comparing base (872cab6) to head (1c0d613).
⚠️ Report is 95 commits behind head on 3.12.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             3.12   #11068      +/-   ##
==========================================
+ Coverage   98.21%   98.24%   +0.02%     
==========================================
  Files         130      130              
  Lines       41588    41612      +24     
  Branches     2281     2286       +5     
==========================================
+ Hits        40847    40881      +34     
+ Misses        563      557       -6     
+ Partials      178      174       -4     
Flag Coverage Δ
CI-GHA 98.13% <100.00%> (+0.01%) ⬆️
OS-Linux 97.87% <100.00%> (+0.01%) ⬆️
OS-Windows 95.47% <100.00%> (+0.49%) ⬆️
OS-macOS 97.08% <100.00%> (+<0.01%) ⬆️
Py-3.10.11 96.96% <100.00%> (?)
Py-3.10.17 97.49% <100.00%> (+0.21%) ⬆️
Py-3.11.12 97.57% <100.00%> (-0.01%) ⬇️
Py-3.11.9 97.05% <100.00%> (?)
Py-3.12.10 97.92% <100.00%> (+0.86%) ⬆️
Py-3.13.3 97.92% <100.00%> (+0.23%) ⬆️
Py-3.9.13 96.86% <100.00%> (+<0.01%) ⬆️
Py-3.9.22 97.38% <100.00%> (+0.20%) ⬆️
Py-pypy7.3.16 86.69% <100.00%> (?)
VM-macos 97.08% <100.00%> (+<0.01%) ⬆️
VM-ubuntu 97.87% <100.00%> (+0.01%) ⬆️
VM-windows 95.47% <100.00%> (+0.49%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bdraco bdraco merged commit 696ae52 into 3.12 May 28, 2025
36 checks passed
@bdraco bdraco deleted the patchback/backports/3.12/e2eb1959237c8bc46a3f0e79a77c2086145d98cf/pr-11054 branch May 28, 2025 21:13
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.

1 participant