fix(realtime): await cancelled background tasks in _cleanup()#3520
fix(realtime): await cancelled background tasks in _cleanup()#3520devteamaegis wants to merge 1 commit into
Conversation
_cleanup_guardrail_tasks() and _cleanup_tool_call_tasks() cancelled tasks
with task.cancel() and immediately cleared the tracking sets, without ever
awaiting the cancelled coroutines. This meant _cleanup() could return
before the cancelled tasks' finally-blocks had run, and cancellation
exceptions or callbacks could fire after the session had already moved on
to model.close().
Fix: make both helpers return the list of pending (cancelled) tasks.
_cleanup() collects them and calls:
await asyncio.gather(*pending_tasks, return_exceptions=True)
before removing the model listener and calling model.close().
return_exceptions=True prevents CancelledError from propagating out of
_cleanup() while still ensuring every task is fully drained.
Closes openai#3334
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11454a70da
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| self._cleanup_guardrail_tasks() + self._cleanup_tool_call_tasks() | ||
| ) | ||
| if pending_tasks: | ||
| await asyncio.gather(*pending_tasks, return_exceptions=True) |
There was a problem hiding this comment.
Avoid awaiting the cleanup caller task
If cleanup is initiated from one of these background tasks, for example an async tool or guardrail that calls await session.close(), the task set still contains asyncio.current_task(), so this gather() attempts to await the task that is currently executing after cancelling it. That can deadlock or recurse through cancellation and prevents the model from being closed; filter the current task out of pending_tasks before awaiting the rest.
Useful? React with 👍 / 👎.
Summary
Fixes #3334 —
RealtimeSession._cleanup()now awaits cancelled guardrail and tool-call tasks before closing the model, so theirfinally-blocks always complete.Root cause
_cleanup_guardrail_tasks()and_cleanup_tool_call_tasks()calledtask.cancel()and immediately cleared the tracking sets, never awaiting the cancelled coroutines:Because
task.cancel()only schedules aCancelledErrorinjection, the tasks'finally-blocks run asynchronously after_cleanup()had already returned. Cancellation exceptions and callbacks could fire after the session had moved on tomodel.close().Fix
Both helpers now return the list of pending tasks.
_cleanup()gathers them before touching the model:return_exceptions=TruepreventsCancelledErrorfrom propagating out of_cleanup()while ensuring every task fully drains.Tests
tests/realtime/test_realtime_cleanup.py— 4 tests:test_cleanup_awaits_guardrail_task_finally_block— finally-block of a cancelled guardrail task runs before_cleanup()returnstest_cleanup_awaits_tool_call_task_finally_block— same for tool-call taskstest_cleanup_awaits_both_task_types_before_model_close— both finally-blocks precedemodel.close()test_cleanup_idempotent_when_no_pending_tasks— no error when task sets are empty