Skip to content

speech_handle: unblock wait_for_playout on interruption#5425

Open
joaquinhuigomez wants to merge 1 commit intolivekit:mainfrom
joaquinhuigomez:fix/speech-handle-interrupt-playout
Open

speech_handle: unblock wait_for_playout on interruption#5425
joaquinhuigomez wants to merge 1 commit intolivekit:mainfrom
joaquinhuigomez:fix/speech-handle-interrupt-playout

Conversation

@joaquinhuigomez
Copy link
Copy Markdown
Contributor

wait_for_playout() awaited only _done_fut, so when user speech interrupted an agent turn during tool preamble playout, any code waiting for playout was stuck until INTERRUPTION_TIMEOUT (5 s) force-killed the task.

Wait on both _done_fut and _interrupt_fut with FIRST_COMPLETED so interruptions resolve immediately instead of deadlocking.

Fixes #5359

wait_for_playout() awaited only _done_fut, so when user speech
interrupted an agent turn during tool preamble playout, any code
waiting for playout was stuck until INTERRUPTION_TIMEOUT (5s)
force-killed the task.

Wait on both _done_fut and _interrupt_fut with FIRST_COMPLETED so
interruptions resolve immediately.

Fixes livekit#5359
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment on lines +186 to +192
done, _ = await asyncio.wait(
[
asyncio.ensure_future(asyncio.shield(self._done_fut)),
asyncio.ensure_future(asyncio.shield(self._interrupt_fut)),
],
return_when=asyncio.FIRST_COMPLETED,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ”΄ Pending wrapper tasks from asyncio.wait are never cancelled, causing task leaks

Each call to wait_for_playout() creates two wrapper tasks via asyncio.ensure_future(asyncio.shield(...)). When asyncio.wait returns after the first future completes, the second wrapper task is left pending and never cancelled or cleaned up. In the common case (speech completes normally without interruption), the shield(self._interrupt_fut) wrapper task will never complete because _interrupt_fut is never resolved for non-interrupted speech β€” it is only set in _cancel() at speech_handle.py:226. These orphaned tasks accumulate over the lifetime of the process and produce Task was destroyed but it is pending! warnings when garbage collected.

Contrast with the established cleanup pattern in the same file

The wait_if_not_interrupted method at livekit-agents/livekit/agents/voice/speech_handle.py:211-219 follows the correct pattern: after asyncio.wait, it explicitly cancels and awaits the pending wrapper future. The new code in wait_for_playout should do the same for its pending wrapper tasks.

Suggested change
done, _ = await asyncio.wait(
[
asyncio.ensure_future(asyncio.shield(self._done_fut)),
asyncio.ensure_future(asyncio.shield(self._interrupt_fut)),
],
return_when=asyncio.FIRST_COMPLETED,
)
shield_done = asyncio.ensure_future(asyncio.shield(self._done_fut))
shield_interrupt = asyncio.ensure_future(asyncio.shield(self._interrupt_fut))
_, pending = await asyncio.wait(
[shield_done, shield_interrupt],
return_when=asyncio.FIRST_COMPLETED,
)
for task in pending:
with contextlib.suppress(asyncio.CancelledError):
task.cancel()
await task
Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SpeechHandle.wait_for_playout() ignores interruption, causing 5s deadlock during tool preamble

1 participant