Prevent cache entry timeout overrides from leaking - #9433

Thanks for the PR. I confirmed the new test fails on master and passes with the fix.

A few things need fixing:

• CI: PEP8_test fails with E501 Line too long (106 > 88) on the changed line in cache.py.

• Scope of the bug: Cache._purge_by_timeout: a per-entry timeout leaks onto the next entry in the same category #9431 and the description say the leak also happens when the category has a timeout. It doesn't: Cache.append() stores timeout or cat['timeout'] on every entry, so entries added to a category with a timeout always carry their own. I ran that scenario (category timeout 100, an earlier entry overridden to 10) on master and the later entry survives. The leak only occurs when the category was registered with timeout=None. Please correct the issue and description.

• Test coverage claim: the description says both insertion orders are covered, but the test only covers one (timed entry first). That is the only failing order, so the test is fine; the description should match.

• Conflict with Fix cache purge removing an extra entry #9423: both PRs add kivy/tests/test_cache.py, so whichever merges second will conflict.

Also, none of the 12 Cache.append() calls in Kivy pass a per-entry timeout, so this only affects third-party code, and #9431 was opened by you the same day as this PR, so I don't see an independent report. For future PRs I'd suggest picking up user-reported issues or items from the Kivy 3.0 project list.

Thanks @ElliotGarbus - all four addressed:

• PEP8: wrapped the line (bff8c88), E501 clean at 88.
• Scope: corrected Cache._purge_by_timeout: a per-entry timeout leaks onto the next entry in the same category #9431 and this description - the leak only occurs for categories registered with timeout=None; as you said, Cache.append stores timeout or cat['timeout'] on every entry, so timed categories are unaffected.
• Coverage: the description now says the test covers only the failing insertion order; the reverse order never leaked.
• test_cache.py collision with Fix cache purge removing an extra entry #9423: noted - whichever of the two lands second, I'll rebase it then.

And understood on selection: user-reported issues or the 3.0 list from here.

@kuzeyron Ready to merge

@kuzeyron I'd keep the explicit is not None here: 0 is a meaningful timeout in Cache (curtime - lastaccess > timeout in _purge_by_timeout purges anything the instant it ages at all), so objtimeout or ... would silently swap a caller's explicit timeout=0 for the category default. The falsy-but-valid 0 is exactly the case the leak fix has to preserve.

Thanks @kuzeyron for the merge, and @ElliotGarbus for the careful review.

Signed-off-by: Charan Rathore [email protected]

Successfully created backport PR for devel-2.3.x:

• [Backport devel-2.3.x] Prevent cache entry timeout overrides from leaking #9519