fix: cleanup pending route handlers on close #1412
Conversation
|
The tests cover the case originally reported in #1402 (comment); however, we should also add a test for the disable interception race reported in #1402 (comment). |
|
Converting to a draft—this worked locally but CI says different! |
…st_should_not_throw_when_continuing_while_page_is_closing
| def __init__(self) -> None: | ||
| self._pending_tasks: List[asyncio.Task] = [] | ||
|
|
||
| def create_task(self, coro: Coroutine) -> asyncio.Task: |
There was a problem hiding this comment.
instead of creating the task here, I'd pass the task over and create it externally.
There was a problem hiding this comment.
What's the advantage of creating it externally? We need to tightly control background tasks, so it seems safest to encapsulate all that's needed to do so like we have done here.
i.e. Don't use asyncio.create_task,—this will create leaks. Use BackgroundTaskTracker.create_task and we'll handle everything for you (like adding a done callback, adding it to pending tasks, and cancelling it eventually).
The thing that breaks us is calling asyncio.create_task and then not tracking it, so we replace the call that you need to use to create the task, and ensure it's being registered and cleaned up properly.
This reverts commit c8d8f4a.
Fixes #1402.