Skip to content

Commit 3e65344

Browse files
authored
fix(agents): release initMutex after warming to restore pool concurrency (#1214)
1 parent 77a8355 commit 3e65344

3 files changed

Lines changed: 101 additions & 1 deletion

File tree

.changeset/afraid-cars-clap.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@livekit/agents": patch
3+
---
4+
5+
fix(agents): release initMutex after warming to restore pool concurrency

agents/src/ipc/proc_pool.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { Throws } from '@livekit/throws-transformer/throws';
55
import { describe, expect, it, vi } from 'vitest';
66
import type { RunningJobInfo } from '../job.js';
77
import { type JobExecutor, JobStatus } from './job_executor.js';
8+
import * as jobProcExecutorModule from './job_proc_executor.js';
89
import { ProcPool } from './proc_pool.js';
910

1011
function createMockExecutor() {
@@ -22,6 +23,13 @@ function createMockExecutor() {
2223
return executor;
2324
}
2425

26+
/** Flush the microtask queue enough times for an async chain to settle. */
27+
async function flushMicrotasks(ticks = 10): Promise<void> {
28+
for (let i = 0; i < ticks; i++) {
29+
await Promise.resolve();
30+
}
31+
}
32+
2533
describe('ProcPool warmed process lock handling', () => {
2634
it('releases lock token from the dequeued warmed process entry', async (): Promise<
2735
Throws<void, Error>
@@ -69,4 +77,84 @@ describe('ProcPool warmed process lock handling', () => {
6977
expect(initUnlock).toHaveBeenCalledTimes(1);
7078
expect(procUnlock).toHaveBeenCalledTimes(1);
7179
});
80+
81+
it('releases initMutex after warming so concurrent procWatchTasks can initialise', async (): Promise<
82+
Throws<void, Error>
83+
> => {
84+
// Regression: initMutex must be released after enqueue, not after join().
85+
// Child procs are one-shot, so holding initMutex through join() serialises
86+
// the pool to effective concurrency 1 regardless of numIdleProcesses.
87+
const pool = new ProcPool('agent', 1, 1000, 1000, undefined, 0, 0);
88+
const initUnlock = vi.fn();
89+
const procUnlock = vi.fn();
90+
91+
let joinResolve: () => void = () => {};
92+
const joinPromise = new Promise<void>((resolve) => {
93+
joinResolve = resolve;
94+
});
95+
const mockProc: JobExecutor = {
96+
...createMockExecutor(),
97+
join: vi.fn(() => joinPromise),
98+
};
99+
100+
const jobProcExecutorSpy = vi
101+
.spyOn(jobProcExecutorModule, 'JobProcExecutor')
102+
.mockImplementation(function MockJobProcExecutor(this: unknown) {
103+
return mockProc as unknown as jobProcExecutorModule.JobProcExecutor;
104+
} as unknown as typeof jobProcExecutorModule.JobProcExecutor);
105+
106+
pool.initMutex.lock = vi.fn(async () => initUnlock);
107+
108+
try {
109+
const watchPromise = pool.procWatchTask(procUnlock);
110+
await flushMicrotasks();
111+
112+
// initMutex released while proc.join() is still pending.
113+
expect(initUnlock).toHaveBeenCalledTimes(1);
114+
expect(pool.warmedProcQueue.items.length).toBe(1);
115+
expect(mockProc.join).toHaveBeenCalledTimes(1);
116+
117+
joinResolve();
118+
await watchPromise;
119+
120+
// finally block must not double-release.
121+
expect(initUnlock).toHaveBeenCalledTimes(1);
122+
expect(procUnlock).not.toHaveBeenCalled();
123+
} finally {
124+
jobProcExecutorSpy.mockRestore();
125+
}
126+
});
127+
128+
it('releases initMutex in finally when initialization fails before enqueue', async (): Promise<
129+
Throws<void, Error>
130+
> => {
131+
const pool = new ProcPool('agent', 1, 1000, 1000, undefined, 0, 0);
132+
const initUnlock = vi.fn();
133+
const procUnlock = vi.fn();
134+
135+
const mockProc: JobExecutor = {
136+
...createMockExecutor(),
137+
initialize: vi.fn(async () => {
138+
throw new Error('simulated initialization failure');
139+
}),
140+
};
141+
142+
const jobProcExecutorSpy = vi
143+
.spyOn(jobProcExecutorModule, 'JobProcExecutor')
144+
.mockImplementation(function MockJobProcExecutor(this: unknown) {
145+
return mockProc as unknown as jobProcExecutorModule.JobProcExecutor;
146+
} as unknown as typeof jobProcExecutorModule.JobProcExecutor);
147+
148+
pool.initMutex.lock = vi.fn(async () => initUnlock);
149+
150+
try {
151+
await pool.procWatchTask(procUnlock);
152+
153+
expect(initUnlock).toHaveBeenCalledTimes(1);
154+
expect(procUnlock).toHaveBeenCalledTimes(1);
155+
expect(pool.warmedProcQueue.items.length).toBe(0);
156+
} finally {
157+
jobProcExecutorSpy.mockRestore();
158+
}
159+
});
72160
});

agents/src/ipc/proc_pool.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ export class ProcPool {
9797
this.executors.push(proc);
9898

9999
const unlock = await this.initMutex.lock();
100+
let initReleased = false;
100101
let procUnlockTransferred = false;
101102
try {
102103
if (this.closed) {
@@ -108,13 +109,19 @@ export class ProcPool {
108109
await proc.initialize();
109110
await this.warmedProcQueue.put({ proc, unlock: procUnlock });
110111
procUnlockTransferred = true;
112+
// Release initMutex after enqueue — holding it through join() serialises
113+
// the pool to concurrency 1 since child procs are one-shot.
114+
unlock();
115+
initReleased = true;
111116
} catch {
112117
// Initialization failed before enqueue, so release the acquired slot immediately.
113118
}
114119

115120
await proc.join();
116121
} finally {
117-
unlock();
122+
if (!initReleased) {
123+
unlock();
124+
}
118125
if (!procUnlockTransferred) {
119126
procUnlock();
120127
}

0 commit comments

Comments
 (0)