-
Notifications
You must be signed in to change notification settings - Fork 330
fix(voice): prevent activity race during session shutdown #2125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@livekit/agents': patch | ||
| --- | ||
|
|
||
| Fix AgentSession shutdown when its active activity changes during asynchronous cleanup. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1281,15 +1281,18 @@ export class AgentSession< | |
| } | ||
| } | ||
|
|
||
| if (this.closing && newActivity === 'start') { | ||
| if (this.closing) { | ||
| this.logger.warn( | ||
| { agentId: this.nextActivity?.agent.id }, | ||
| 'Session is closing, skipping start of next activity', | ||
| { agentId: this.nextActivity?.agent.id, transition: newActivity }, | ||
| 'Session is closing, skipping activity transition', | ||
| ); | ||
| if (reusableResources) { | ||
| await cleanupReusableResources(reusableResources, this.logger); | ||
| reusableResources = undefined; | ||
| } | ||
| if (newActivity === 'resume') { | ||
| await this.nextActivity?.close(); | ||
| } | ||
| this.nextActivity = undefined; | ||
| this.activity = undefined; | ||
| return; | ||
|
|
@@ -1704,25 +1707,26 @@ export class AgentSession< | |
| this._onAecWarmupExpired(); | ||
| this.off(AgentSessionEventTypes.UserInputTranscribed, this._onUserInputTranscribed); | ||
|
|
||
| if (this.activity) { | ||
| const activity = this.activity; | ||
| if (activity) { | ||
| if (!drain) { | ||
| try { | ||
| await this.activity.interrupt({ force: true }).await; | ||
| await activity.interrupt({ force: true }).await; | ||
| } catch (error) { | ||
| this.logger.warn({ error }, 'Error interrupting activity'); | ||
| } | ||
| } | ||
|
|
||
| await this.activity.drain(); | ||
| await activity.drain(); | ||
| // wait any uninterruptible speech to finish | ||
| await this.activity.currentSpeech?.waitForPlayout(); | ||
| await activity.currentSpeech?.waitForPlayout(); | ||
|
|
||
| if (reason !== CloseReason.ERROR) { | ||
| this.activity.commitUserTurn({ audioDetached: true, throwIfNotReady: false }); | ||
| activity.commitUserTurn({ audioDetached: true, throwIfNotReady: false }); | ||
| } | ||
|
|
||
| try { | ||
| this.activity.detachAudioInput(); | ||
| activity.detachAudioInput(); | ||
| } catch (error) { | ||
| // Ignore detach errors during cleanup - source may not have been set | ||
| } | ||
|
|
@@ -1738,7 +1742,7 @@ export class AgentSession< | |
| this.output.audio = null; | ||
| this.output.transcription = null; | ||
|
|
||
| await this.activity?.close(); | ||
| await activity?.close(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an Useful? React with 👍 / 👎. |
||
| this.activity = undefined; | ||
|
|
||
| const sessionToolsets = this._toolCtx.toolsets; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // SPDX-FileCopyrightText: 2026 LiveKit, Inc. | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
| import { initializeLogger } from '../log.js'; | ||
| import { Agent } from './agent.js'; | ||
| import type { AgentActivity } from './agent_activity.js'; | ||
| import { AgentSession } from './agent_session.js'; | ||
| import { AgentSessionEventTypes } from './events.js'; | ||
|
|
||
| describe('AgentSession close activity race', () => { | ||
| initializeLogger({ pretty: false, level: 'silent' }); | ||
|
|
||
| it('finishes closing the captured activity when the session activity changes during drain', async () => { | ||
| const session = new AgentSession(); | ||
| await session.start({ agent: new Agent({ instructions: 'test' }) }); | ||
|
|
||
| const internals = session as unknown as { activity?: AgentActivity }; | ||
| const activity = internals.activity!; | ||
|
|
||
| let releaseDrain!: () => void; | ||
| const drainGate = new Promise<void>((resolve) => { | ||
| releaseDrain = resolve; | ||
| }); | ||
| let markDrainStarted!: () => void; | ||
| const drainStarted = new Promise<void>((resolve) => { | ||
| markDrainStarted = resolve; | ||
| }); | ||
|
|
||
| vi.spyOn(activity, 'drain').mockImplementation(async () => { | ||
| markDrainStarted(); | ||
| await drainGate; | ||
| return undefined; | ||
| }); | ||
| const activityClose = vi.spyOn(activity, 'close'); | ||
| const onClose = vi.fn(); | ||
| session.on(AgentSessionEventTypes.Close, onClose); | ||
|
|
||
| const closePromise = session.close(); | ||
| await drainStarted; | ||
|
|
||
| internals.activity = undefined; | ||
| releaseDrain(); | ||
|
|
||
| await expect(closePromise).resolves.toBeUndefined(); | ||
| expect(activityClose).toHaveBeenCalledOnce(); | ||
| expect(onClose).toHaveBeenCalledOnce(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an
AgentTaskcompletes after shutdown has setclosing, this resume transition runs from the task that is still registered in the paused activity'sspeechTasks. Awaiting that activity'sclose()here makesAgentActivity.close()cancel and wait for the calling task itself; the task cannot finish until_updateActivity()returns, so shutdown stalls for the five-secondREPLY_TASK_CANCEL_TIMEOUT(and may exceed the host's shutdown grace period). Defer this close until the caller has unwound, or exclude the current task from the close wait.Useful? React with 👍 / 👎.