diff --git a/src/main/speech/model-manager.test.ts b/src/main/speech/model-manager.test.ts index 6c25e9a5e..5e43cc528 100644 --- a/src/main/speech/model-manager.test.ts +++ b/src/main/speech/model-manager.test.ts @@ -2,16 +2,25 @@ import { createHash } from 'crypto' import { mkdtempSync, rmSync, writeFileSync } from 'fs' import { tmpdir } from 'os' import { join } from 'path' -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { SPEECH_MODEL_CATALOG } from './model-catalog' import { ModelManager } from './model-manager' +const { spawnMock } = vi.hoisted(() => ({ + spawnMock: vi.fn() +})) + vi.mock('electron', () => ({ app: { getPath: () => '/tmp/orca-speech-models-test' } })) +vi.mock('child_process', async () => { + const actual = await vi.importActual('child_process') + return { ...(actual as Record), spawn: spawnMock } +}) + type ModelManagerInternals = { verifyArchiveSha256: (archivePath: string, expectedSha256: string) => Promise downloadFile: ( @@ -21,9 +30,19 @@ type ModelManagerInternals = { modelId: string, isAborted: () => boolean ) => Promise + extractArchive: ( + archivePath: string, + destDir: string, + modelId: string, + isAborted: () => boolean + ) => Promise } describe('ModelManager', () => { + beforeEach(() => { + spawnMock.mockReset() + }) + it('requires pinned SHA-256 hashes for every catalog archive', () => { for (const manifest of SPEECH_MODEL_CATALOG) { expect(manifest.archiveSha256).toMatch(/^[a-f0-9]{64}$/) @@ -65,4 +84,59 @@ describe('ModelManager', () => { rmSync(dir, { recursive: true, force: true }) } }) + + it('clears extraction abort polling when the child does not close', async () => { + vi.useFakeTimers() + const dir = mkdtempSync(join(tmpdir(), 'orca-model-manager-')) + try { + const handlers: Record void)[]> = { + close: [], + error: [] + } + const stderrHandlers: ((chunk: Buffer) => void)[] = [] + const child = { + stderr: { + on: vi.fn((_event: string, cb: (chunk: Buffer) => void) => { + stderrHandlers.push(cb) + return child.stderr + }), + off: vi.fn((_event: string, cb: (chunk: Buffer) => void) => { + const index = stderrHandlers.indexOf(cb) + if (index !== -1) { + stderrHandlers.splice(index, 1) + } + return child.stderr + }) + }, + kill: vi.fn(), + on: vi.fn((event: string, cb: (arg?: unknown) => void) => { + handlers[event]?.push(cb) + return child + }), + off: vi.fn((event: string, cb: (arg?: unknown) => void) => { + handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? [] + return child + }) + } + spawnMock.mockReturnValue(child) + const manager = new ModelManager(dir) as unknown as ModelManagerInternals + + const extraction = manager.extractArchive(join(dir, 'model.tar.bz2'), dir, 'm', () => true) + const rejection = expect(extraction).rejects.toThrow('Aborted') + await vi.advanceTimersByTimeAsync(250) + await rejection + + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + expect(child.kill).toHaveBeenCalledTimes(1) + expect(handlers.close).toHaveLength(0) + expect(handlers.error).toHaveLength(0) + expect(stderrHandlers).toHaveLength(0) + + vi.advanceTimersByTime(1000) + expect(child.kill).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + rmSync(dir, { recursive: true, force: true }) + } + }) }) diff --git a/src/main/speech/model-manager.ts b/src/main/speech/model-manager.ts index 9c25c728b..e31fad215 100644 --- a/src/main/speech/model-manager.ts +++ b/src/main/speech/model-manager.ts @@ -367,36 +367,66 @@ export class ModelManager { ) let stderr = '' - child.stderr?.on('data', (chunk: Buffer) => { - stderr += chunk.toString() - }) - - const timeout = setTimeout(() => { - child.kill('SIGKILL') - reject(new Error('Extraction timed out after 10 minutes')) - }, 600_000) - const abortPoll = setInterval(() => { - if (isAborted()) { - child.kill('SIGKILL') - reject(new Error('Aborted')) + let settled = false + let timeout: ReturnType | null = null + let abortPoll: ReturnType | null = null + const cleanup = (): void => { + if (timeout) { + clearTimeout(timeout) + timeout = null } - }, 250) - - child.on('close', (code) => { - clearTimeout(timeout) - clearInterval(abortPoll) + if (abortPoll) { + clearInterval(abortPoll) + abortPoll = null + } + child.stderr?.off('data', onStderrData) + child.off('close', onClose) + child.off('error', onError) + } + const fail = (error: Error, killChild = false): void => { + if (settled) { + return + } + settled = true + cleanup() + if (killChild) { + child.kill('SIGKILL') + } + reject(error) + } + const onStderrData = (chunk: Buffer): void => { + stderr += chunk.toString() + } + const onClose = (code: number | null): void => { + if (settled) { + return + } + settled = true + cleanup() if (code === 0) { resolve() } else { reject(new Error(`tar exited with code ${code}: ${stderr.slice(0, 500)}`)) } - }) + } + const onError = (err: Error): void => { + fail(err) + } - child.on('error', (err) => { - clearTimeout(timeout) - clearInterval(abortPoll) - reject(err) - }) + child.stderr?.on('data', onStderrData) + timeout = setTimeout(() => { + fail(new Error('Extraction timed out after 10 minutes'), true) + }, 600_000) + abortPoll = setInterval(() => { + if (isAborted()) { + // Why: if the extraction child wedges and never emits close/error, + // the abort poller must still clear itself when we reject. + fail(new Error('Aborted'), true) + } + }, 250) + + child.on('close', onClose) + child.on('error', onError) }) }