Conversation
References to tasks scheduled with asyncio.create_task() must be held; if they aren't held, tasks may disappear mid-execution. https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR addresses asyncio “fire-and-forget” task lifetime issues by ensuring tasks created via asyncio.create_task() are strongly referenced, and it enables Ruff’s RUF006 rule to prevent regressions.
Changes:
- Enable Ruff lint rule
RUF006(store a reference to the return value ofasyncio.create_task). - Keep strong references to background tasks in several runtime/server components to prevent premature garbage collection.
- Introduce module-level background task registries for certain WebSocket-related fire-and-forget tasks.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Enables Ruff RUF006 to enforce holding references to asyncio.create_task() results. |
| marimo/_utils/file_watcher.py | Stores the polling watcher task on the instance to prevent GC mid-poll. |
| marimo/_server/api/middleware.py | Stores a reference to the inactivity monitor task to prevent GC. |
| marimo/_server/api/endpoints/ws_endpoint.py | Tracks background close tasks in a module-level set to prevent GC mid-flight. |
| marimo/_server/api/endpoints/ws/ws_kernel_ready.py | Tracks background RTC doc creation tasks in a module-level set to prevent GC mid-flight. |
Comments suppressed due to low confidence (1)
marimo/_utils/file_watcher.py:80
PollingFileWatchernow keeps a strong reference to the polling task, butstop()only flips_runningand never cancels/clears the task. If_poll()raises (e.g., callback error), the exception may be silently hidden indefinitely because the task is never awaited/GC’d. Consider adding a done-callback to (1) log/retrieve exceptions and (2) clearself._task, and cancel the task instop()when it’s still running.
def start(self) -> None:
self._running = True
# Hold a strong reference so the task isn't GC'd mid-poll.
self._task = self.loop.create_task(self._poll())
def stop(self) -> None:
self._running = False
| self.timeout_duration_minutes = timeout_duration_minutes | ||
|
|
||
| asyncio.create_task(self.monitor()) | ||
| # Hold a strong reference so the monitor task isn't GC'd. | ||
| self._monitor_task = asyncio.create_task(self.monitor()) | ||
|
|
There was a problem hiding this comment.
TimeoutMiddleware holds onto the monitor task, but the task is never awaited and no done-callback retrieves/logs exceptions. With a strong reference, a failure inside monitor() can become silent (no “Task exception was never retrieved” warning). Consider adding a done-callback that logs task.exception() (handling CancelledError) and/or cancelling the task during shutdown/lifespan teardown.
| task = asyncio.create_task( | ||
| self._safe_close( | ||
| WebSocketCodes.NORMAL_CLOSE, "MARIMO_SHUTDOWN" | ||
| ) | ||
| ) | ||
| _background_tasks.add(task) | ||
| task.add_done_callback(_background_tasks.discard) |
There was a problem hiding this comment.
The done-callback only discards the task from _background_tasks. If _safe_close() raises, the exception will remain unobserved and may later emit a noisy “Task exception was never retrieved” warning. Consider a done-callback that both discards the task and explicitly retrieves/logs task.exception() (ignoring cancellations).
| task = asyncio.create_task( | ||
| doc_manager.create_doc(file_key, cell_ids, codes) | ||
| ) | ||
| _background_tasks.add(task) | ||
| task.add_done_callback(_background_tasks.discard) |
There was a problem hiding this comment.
The background task is discarded on completion, but exceptions aren’t retrieved/logged. If doc_manager.create_doc(...) fails, this can surface as a late “Task exception was never retrieved” warning with little context. Consider adding a done-callback that discards the task and logs task.exception() (handling cancellations).
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.2-dev70 |
References to tasks scheduled with
asyncio.create_task()must be held; if they aren't held, tasks may disappear mid-execution.https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task
This PR also enables
RUF006to ensure that this does not happen again.