fix(realtime): await background tasks during cleanup#3656
fix(realtime): await background tasks during cleanup#3656gyx09212214-prog wants to merge 3 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be773870d4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if tasks_to_await: | ||
| await asyncio.gather(*tasks_to_await, return_exceptions=True) | ||
|
|
||
| tasks.clear() |
There was a problem hiding this comment.
When cleanup is awaiting cancellation finalizers, the session is still accepting model events because the listener is removed only after these awaits. If a transcript/function_call event or approval path enqueues another task after list(tasks) is taken, tasks.clear() removes that new live task from tracking without cancelling or awaiting it, so it can continue after close() and send events to a model that is being closed. Consider marking the session closing/removing the listener before awaiting, or only discarding the snapshot and looping until no tracked tasks remain.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14d12e901f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._cleanup_tool_call_tasks() | ||
|
|
||
| # Remove ourselves as a listener | ||
| self._model.remove_listener(self) |
There was a problem hiding this comment.
Removing the listener here does not stop model events that already copied this listener before close() began; fresh evidence is OpenAIRealtimeModel._emit_event in src/agents/realtime/openai_realtime.py lines 624-626, which iterates over list(self._listeners) and then awaits listener.on_event(event). When close() races with an already-dispatched function_call and _tool_call_tasks is still empty, _cleanup_tool_call_tasks() returns before that in-flight on_event reaches _enqueue_tool_call_task, so the newly added tool task is never cancelled or awaited and can continue after the session is closed.
Useful? React with 👍 / 👎.
seratch
added
duplicate
2 participants
Summary
Fixes #3334.
RealtimeSession._cleanup()now cancels and awaits pending guardrail and tool-call background tasks before clearing their tracking sets and continuing model shutdown. This ensures cancellationfinallyblocks finish before cleanup returns, and avoids processing task cancellation after the session has already moved on to closing the model.What changed
return_exceptions=True._cleanup(), and asserts both tasks are done and theirfinallyblocks ran before cleanup returns.Test plan
python -m pytest tests/realtime/test_session.py -qpython -m ruff check src/agents/realtime/session.py tests/realtime/test_session.pypython -m ruff format --check src/agents/realtime/session.py tests/realtime/test_session.pypython -m py_compile src/agents/realtime/session.py tests/realtime/test_session.pygit diff --check