fix: resolve voice pipeline deadlock on TTS error and retrieve background task exceptions#3921
fix: resolve voice pipeline deadlock on TTS error and retrieve background task exceptions#3921AAliKKhan wants to merge 4 commits into
Conversation
…input deep copy, fix unretrieved task exception
![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: 036cf41bf6
ℹ️ 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".
|
|
||
| # Signal completion for whole session because of error | ||
| await local_queue.put(VoiceStreamEventLifecycle(event="session_ended")) | ||
| await self._add_error(e) |
There was a problem hiding this comment.
When a later TTS segment fails faster than an earlier segment, this puts the terminal error directly on the public queue instead of going through _ordered_tasks/_dispatch_audio. Because stream() stops as soon as it reads VoiceStreamEventError, consumers can miss already-started earlier audio and lifecycle events whenever concurrent segment synthesis completes out of order; route the terminal error through the ordered path, or otherwise wait for earlier local queues before publishing it.
AGENTS.md reference: AGENTS.md:L109-L110
Useful? React with 👍 / 👎.
| for task in all_tasks: | ||
| if task.done() and not task.cancelled(): | ||
| if task.exception(): | ||
| self._stored_exception = task.exception() | ||
| break |
There was a problem hiding this comment.
When a TTS segment fails during a pipeline run, the segment task and text_generation_task can both complete with exceptions because the producer re-raises the gather failure. This loop still scans the segment tasks first and breaks on the first exception, so it never calls exception() on text_generation_task; the original “Task exception was never retrieved” warning remains for that failure path. Continue retrieving all done task exceptions while only preserving the first stored exception.
AGENTS.md reference: AGENTS.md:L109-L110
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b0e7381b5
ℹ️ 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".
| await self._add_error(e) | ||
| await local_queue.put(None) | ||
| if self.text_generation_task is not None and not self.text_generation_task.done(): | ||
| self.text_generation_task.cancel() |
There was a problem hiding this comment.
When a TTS segment fails while the streamed multi-turn producer is between turns or waiting on transcription/workflow, this cancellation makes process_turns() enter its finally to close the STT session, but stream() has already received the public error and then _cleanup_tasks() can cancel the same producer again while transcription_session.close()/output._done() is awaited. That second cancellation can interrupt cleanup, leaving the STT connection or trace unfinished; let the producer run its finally to completion, or shield that cleanup, instead of canceling it from the TTS task.
AGENTS.md reference: AGENTS.md:L110-L110
Useful? React with 👍 / 👎.
2 participants
This pull request fixes three issues in the voice pipeline error handling:
Deadlock when TTS streaming fails — When _stream_audio\ encounters an exception, the error handler now puts the error on the public queue (via _add_error) and cancels the producer task instead of putting \session_ended\ on the local queue. Previously, \stream()\ would see the \session_ended\ event, enter the \�syncio.shield\ path, and hang indefinitely waiting for a producer that hadn't finished.
Background task exception never retrieved — _check_errors\ only inspected _stream_audio\ tasks, missing \ ext_generation_task. Added it to the check with a \ ask.cancelled()\ guard to prevent \CancelledError\ from being raised.
No \None\ sentinel on error — When a TTS task fails, the dispatcher's inner loop needed a \None\ sentinel to break out of the segment's local queue processing.