Repository navigation
fix(memory): restart the cleanup task on a new event loop and add aclose() - #215
Merged
Merged
Conversation
…ose() The cleanup task stayed bound to the loop of the first cache call. Once that loop closed without cancelling it, the task looked pending forever and was never started on the new loop. The backend now restarts the task when a call runs on a different loop, cancelling the old one if its loop is still open (thread-safely when it is not the running loop). aclose() cancels the task and waits for it, for use in a lifespan shutdown. stop_cleanup() keeps its synchronous behaviour. Closes #181
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #181
Problem
MemoryBackendstarted its cleanup task on the event loop of the first cache call and never looked at it again. If that loop was closed without cancelling the task (a manually managed loop, some test runners, a server restart inside one process), the task stayed "pending" forever. Every later call saw a task that was not done, so no cleanup ran on the new loop and expired entries piled up.Change
_ensure_cleanup_started()checks that the existing task belongs to the running loop. If not, it cancels the old task and starts a new one on the current loop._cancel()helper: a task on a closed loop is left alone (cancelling it would raise), a task on the running loop is cancelled directly, and a task on another open loop is cancelled withcall_soon_threadsafe.async def aclose(): cancels the task and waits until it has finished, for use in a lifespan shutdown. It only waits for a task on the running loop.stop_cleanup()stays synchronous and unchanged in behaviour. It now uses the same helper, and its docstring points toaclose().BACKENDS.md): a paragraph on the loop restart and a lifespan example withaclose().aclose()and a Fixed entry for the restart.Tests
Four new tests in
tests/backends/test_memory.py:aclose()waits for the task;aclose()only cancels, and does not wait for, a task on another loop.Each fix point was mutation-checked by reverting it: ignoring the loop, removing the closed-loop guard, not waiting in
aclose(), and not cancelling across loops. Each revert fails the matching new tests.Local checks: ruff,
mypy --strict, the full suite against live Redis and Memcached (882 passed), andzensical build --strictfor both languages.