mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-21 18:44:30 -07:00
fix(core): dispose Scheduler to prevent McpProgress listener leak (#24870)
This commit is contained in:
@@ -71,6 +71,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
Scheduler: class {
|
||||
schedule = mockSchedulerSchedule;
|
||||
cancelAll = vi.fn();
|
||||
dispose = vi.fn();
|
||||
},
|
||||
isTelemetrySdkInitialized: vi.fn().mockReturnValue(true),
|
||||
ChatRecordingService: MockChatRecordingService,
|
||||
|
||||
@@ -187,6 +187,7 @@ export async function runNonInteractive(
|
||||
};
|
||||
|
||||
let errorToHandle: unknown | undefined;
|
||||
let scheduler: Scheduler | undefined;
|
||||
try {
|
||||
consolePatcher.patch();
|
||||
|
||||
@@ -215,7 +216,7 @@ export async function runNonInteractive(
|
||||
});
|
||||
|
||||
const geminiClient = config.getGeminiClient();
|
||||
const scheduler = new Scheduler({
|
||||
scheduler = new Scheduler({
|
||||
context: config,
|
||||
messageBus: config.getMessageBus(),
|
||||
getPreferredEditor: () => undefined,
|
||||
@@ -528,6 +529,7 @@ export async function runNonInteractive(
|
||||
// Cleanup stdin cancellation before other cleanup
|
||||
cleanupStdinCancellation();
|
||||
|
||||
scheduler?.dispose();
|
||||
consolePatcher.cleanup();
|
||||
coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback);
|
||||
}
|
||||
|
||||
@@ -71,6 +71,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
Scheduler: class {
|
||||
schedule = mockSchedulerSchedule;
|
||||
cancelAll = vi.fn();
|
||||
dispose = vi.fn();
|
||||
},
|
||||
isTelemetrySdkInitialized: vi.fn().mockReturnValue(true),
|
||||
ChatRecordingService: MockChatRecordingService,
|
||||
|
||||
@@ -184,6 +184,7 @@ export async function runNonInteractive({
|
||||
};
|
||||
|
||||
let errorToHandle: unknown | undefined;
|
||||
let scheduler: Scheduler | undefined;
|
||||
let abortSession = () => {};
|
||||
try {
|
||||
consolePatcher.patch();
|
||||
@@ -215,7 +216,7 @@ export async function runNonInteractive({
|
||||
});
|
||||
|
||||
const geminiClient = config.getGeminiClient();
|
||||
const scheduler = new Scheduler({
|
||||
scheduler = new Scheduler({
|
||||
context: config,
|
||||
messageBus: config.getMessageBus(),
|
||||
getPreferredEditor: () => undefined,
|
||||
@@ -612,6 +613,7 @@ export async function runNonInteractive({
|
||||
cleanupStdinCancellation();
|
||||
abortController.signal.removeEventListener('abort', abortSession);
|
||||
|
||||
scheduler?.dispose();
|
||||
consolePatcher.cleanup();
|
||||
coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback);
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
vi.mock('../scheduler/scheduler.js', () => ({
|
||||
Scheduler: vi.fn().mockImplementation(() => ({
|
||||
schedule: vi.fn().mockResolvedValue([{ status: 'success' }]),
|
||||
dispose: vi.fn(),
|
||||
})),
|
||||
}));
|
||||
|
||||
@@ -125,6 +126,57 @@ describe('agent-scheduler', () => {
|
||||
expect(schedulerConfig.toolRegistry).not.toBe(mainRegistry);
|
||||
});
|
||||
|
||||
it('should dispose the scheduler after schedule completes', async () => {
|
||||
const mockConfig = {
|
||||
getPromptRegistry: vi.fn(),
|
||||
getResourceRegistry: vi.fn(),
|
||||
messageBus: mockMessageBus,
|
||||
toolRegistry: mockToolRegistry,
|
||||
} as unknown as Mocked<Config>;
|
||||
|
||||
const options = {
|
||||
schedulerId: 'subagent-1',
|
||||
toolRegistry: mockToolRegistry as unknown as ToolRegistry,
|
||||
signal: new AbortController().signal,
|
||||
};
|
||||
|
||||
await scheduleAgentTools(mockConfig as unknown as Config, [], options);
|
||||
|
||||
const schedulerInstance = vi.mocked(Scheduler).mock.results[0].value;
|
||||
expect(schedulerInstance.dispose).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('should dispose the scheduler even when schedule throws', async () => {
|
||||
const scheduleError = new Error('schedule failed');
|
||||
vi.mocked(Scheduler).mockImplementationOnce(
|
||||
() =>
|
||||
({
|
||||
schedule: vi.fn().mockRejectedValue(scheduleError),
|
||||
dispose: vi.fn(),
|
||||
}) as unknown as Scheduler,
|
||||
);
|
||||
|
||||
const mockConfig = {
|
||||
getPromptRegistry: vi.fn(),
|
||||
getResourceRegistry: vi.fn(),
|
||||
messageBus: mockMessageBus,
|
||||
toolRegistry: mockToolRegistry,
|
||||
} as unknown as Mocked<Config>;
|
||||
|
||||
const options = {
|
||||
schedulerId: 'subagent-1',
|
||||
toolRegistry: mockToolRegistry as unknown as ToolRegistry,
|
||||
signal: new AbortController().signal,
|
||||
};
|
||||
|
||||
await expect(
|
||||
scheduleAgentTools(mockConfig as unknown as Config, [], options),
|
||||
).rejects.toThrow('schedule failed');
|
||||
|
||||
const schedulerInstance = vi.mocked(Scheduler).mock.results[0].value;
|
||||
expect(schedulerInstance.dispose).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('should create an AgentLoopContext that has a defined .config property', async () => {
|
||||
const mockConfig = {
|
||||
getPromptRegistry: vi.fn(),
|
||||
|
||||
@@ -85,5 +85,9 @@ export async function scheduleAgentTools(
|
||||
onWaitingForConfirmation,
|
||||
});
|
||||
|
||||
return scheduler.schedule(requests, signal);
|
||||
try {
|
||||
return await scheduler.schedule(requests, signal);
|
||||
} finally {
|
||||
scheduler.dispose();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user