From 8500a2165823de9c9f4332c98ae5a9010a384ef1 Mon Sep 17 00:00:00 2001 From: Huanqi Cao Date: Mon, 3 Aug 2026 20:40:10 +0800 Subject: [PATCH] fix(picker): pointer-width vtable offsets, COM apartment pairing, unconditional abort budget, and the full failure chain Review round two on the in-process dialog: - Vtable slots and out-pointers use koffi.sizeof('void *') instead of a hardcoded 8 - win32-ia32 (which Node and koffi both ship) would have read method pointers from the wrong address and crashed in-process before any fallback could run. - runFolderDialog pairs every successful (incl. S_FALSE) CoInitializeEx with CoUninitialize in the outermost finally, releasing the dialog first; a failed init is deliberately unpaired. Pinned across fake-bindings and mocked-koffi suites. - The abort close budget starts unconditionally: a worker hung before the showing notice (koffi import or COM init) now ends in terminate instead of a dangling promise; WM_CLOSE posting still waits for the thread id. - A triple miss (dialog + pwsh + 5.1) surfaces an AggregateError carrying all three causes - the in-process tier's reason was previously unrecoverable from the final PowerShell error. - The stray '=>{ ' formatter artifacts are normalized to real blocks. Both stale note claims from the review are fixed: the DPI note's Consequences no longer claims an ENOENT classification or zero new dependencies, and the 2026-07-27 picker note's Windows bullet now names the in-process primary and keeps the PowerShell chain as fallback (both languages, pairings re-recorded). --- ...26-08-01-windows-picker-pwsh-dpi.i18n.yaml | 4 +- .../2026-08-01-windows-picker-pwsh-dpi.md | 2 +- .../2026-08-01-windows-picker-pwsh-dpi.zh.md | 2 +- ...ative-workspace-directory-picker.i18n.yaml | 4 +- ...07-27-native-workspace-directory-picker.md | 4 +- ...27-native-workspace-directory-picker.zh.md | 4 +- .../src/native-picker.ts | 19 +++++++++- .../src/win32-dialog-bindings.ts | 12 +++++- .../src/win32-dialog-logic.ts | 34 +++++++++++------ .../src/win32-dialog-worker.ts | 4 +- .../src/win32-dialog.ts | 37 ++++++++++++++----- .../tests/native-picker.spec.ts | 12 +++++- .../tests/win32-dialog-bindings.spec.ts | 8 +++- .../tests/win32-dialog-logic.spec.ts | 27 +++++++++----- .../tests/win32-dialog.spec.ts | 24 ++++++++++-- 15 files changed, 147 insertions(+), 50 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.i18n.yaml index d9f46441ac..9a86e1f729 100644 --- a/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.md -2026-08-01-windows-picker-pwsh-dpi.md: 1d9fd0a1b445a77b478f033d56169d6166c2b1bf -2026-08-01-windows-picker-pwsh-dpi.zh.md: 245991e7c8d081f3c91724c0dae1142d80883c1d +2026-08-01-windows-picker-pwsh-dpi.md: a941d5ea6e150d74fa2fa4dbd93b7e6b58a78eff +2026-08-01-windows-picker-pwsh-dpi.zh.md: 8383240c219d701aa9a8728cf1a7fb7f3a7c5433 diff --git a/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.md b/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.md index 1d9fd0a1b4..a941d5ea6e 100644 --- a/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.md +++ b/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.md @@ -22,5 +22,5 @@ The PowerShell chain is now the FALLBACK tier below the in-process koffi dialog ## Consequences - Machines with PowerShell 7 get the modern folder picker; 5.1-only machines keep the legacy tree — now sharp — and the package README's Known Limitations documents the gap. -- No new packages or runtime dependencies; the fallback reuses the existing `ENOENT` classification and abort propagation. +- The PowerShell chain itself adds no packages or dependencies (koffi and tsx arrived with the in-process primary and belong to its note); the pwsh→5.1 hop triggers on ANY non-abort pwsh failure — no `ENOENT` classification remains on the win32 path — while abort propagation is unchanged. - The command boundary (`DirectoryPickerRunner`) pins the spawn order and script content in unit tests; real dialog rendering remains a manual Windows check, as before. diff --git a/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.zh.md b/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.zh.md index 245991e7c8..8383240c21 100644 --- a/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-08-01-windows-picker-pwsh-dpi.zh.md @@ -22,5 +22,5 @@ PowerShell 链现在是进程内 koffi 对话框之下的回退层(见[进程 ## 后果 - 装有 PowerShell 7 的机器获得现代文件夹选择器;只有 5.1 的机器保留旧版树——但现在清晰了——包 README 的已知限制记录了该差距。 -- 无新增包或运行时依赖;回退复用既有的 `ENOENT` 分类与中止传播。 +- PowerShell 链本身不新增任何包或依赖(koffi 与 tsx 随进程内主层引入,归属其 Note);pwsh→5.1 的跳转在 pwsh 的任何非中止失败上触发——win32 路径上已不存在 `ENOENT` 分类——中止传播不变。 - 命令边界(`DirectoryPickerRunner`)在单元测试中固定启动顺序与脚本内容;真实对话框渲染仍与以前一样属于手动 Windows 检查。 diff --git a/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.i18n.yaml b/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.i18n.yaml index 12cb856fe2..e177c663cc 100644 --- a/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.md -2026-07-27-native-workspace-directory-picker.md: c18b4263d4e97290d69ac229e7423558bdb4c3b1 -2026-07-27-native-workspace-directory-picker.zh.md: 7267516d7eea4b3cfecc2ca8f18305896eaffede +2026-07-27-native-workspace-directory-picker.md: 45fa77b5519179e006f9109846a1602e6e22a6e2 +2026-07-27-native-workspace-directory-picker.zh.md: 2d6800d20b1f0dfe0b20ac9a5c90037599ece32a diff --git a/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.md b/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.md index c18b4263d4..45fa77b551 100644 --- a/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.md +++ b/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.md @@ -27,10 +27,10 @@ The workspace manager must upsert the returned workspace before the selection ca The native dialog RPC is accepted only from a loopback socket with same-origin browser metadata. The RPC does not use the default 30-second request timeout because a system dialog may remain open indefinitely; caller and connection aborts still propagate to the platform process. -Platform adapters invoke native tools without a shell: +Platform adapters open the dialog without a shell — spawned native tools on POSIX, an in-process COM conversation on Windows: - macOS: `osascript` and the system folder chooser. -- Windows: `pwsh` (PowerShell 7) in STA mode with a Windows PowerShell 5.1 fallback, always DPI-aware ([picker fix](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md)). +- Windows: the in-process koffi `IFileOpenDialog` worker with per-monitor-v2 DPI ([in-process dialog note](2026-08-02-win32-in-process-folder-dialog.md)); the PowerShell chain (`pwsh` in STA mode, then Windows PowerShell 5.1, both DPI-corrected) remains the fallback ([picker fix](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md)). - Linux: `zenity`, with `kdialog` as a fallback when Zenity is unavailable. ## Alternatives considered diff --git a/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.zh.md b/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.zh.md index 7267516d7e..2d6800d20b 100644 --- a/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.zh.md +++ b/.agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.zh.md @@ -27,10 +27,10 @@ Status: implemented 只有来自回环套接字、且携带同源浏览器元数据的请求才能调用原生对话框 RPC。该 RPC 不使用默认的 30 秒请求超时,因为系统对话框可能无限期保持打开;调用方中止或连接中止仍会传递至平台进程。 -平台适配器不经 shell,直接调用原生工具: +平台适配器不经 shell 打开对话框——POSIX 上 spawn 原生工具,Windows 上是进程内 COM 会话: - macOS:`osascript` 和系统文件夹选择器。 -- Windows:采用 STA 模式的 `pwsh`(PowerShell 7),并以 Windows PowerShell 5.1 回退,且始终 DPI aware(见[选择器修复](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md))。 +- Windows:进程内 koffi `IFileOpenDialog` worker,带 per-monitor-v2 DPI(见[进程内对话框 Note](2026-08-02-win32-in-process-folder-dialog.md));PowerShell 链(STA 模式的 `pwsh`,再到 Windows PowerShell 5.1,均已修正 DPI)保留为回退(见[选择器修复](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md))。 - Linux:使用 `zenity`;Zenity 不可用时回退到 `kdialog`。 ## 考虑过的替代方案 diff --git a/packages/host/directory-picker-native/src/native-picker.ts b/packages/host/directory-picker-native/src/native-picker.ts index 6cabf44ddb..4b4d75fe32 100644 --- a/packages/host/directory-picker-native/src/native-picker.ts +++ b/packages/host/directory-picker-native/src/native-picker.ts @@ -72,10 +72,12 @@ export async function pickNativeDirectory( // support. Any non-abort failure (koffi unavailable, ancient Windows, COM // refusal) falls back to the PowerShell chain below. const pickDialog = internals.pickWin32Dialog ?? pickWin32Directory + let dialogError: unknown try { return await pickDialog(signal) } catch (error: unknown) { rethrowIfAborted(signal, error) + dialogError = error } // PowerShell fallback: PowerShell 7 renders the modern IFileDialog folder @@ -100,14 +102,27 @@ export async function pickNativeDirectory( ' [Console]::WriteLine($dialog.SelectedPath)', '}', ].join('; ') + let pwshError: unknown try { const result = await run('pwsh.exe', ['-NoProfile', '-STA', '-Command', script], signal) return outputPath(result.stdout) } catch (error: unknown) { rethrowIfAborted(signal, error) + pwshError = error + } + try { + const result = await run('powershell.exe', ['-NoProfile', '-STA', '-Command', script], signal) + return outputPath(result.stdout) + } catch (error: unknown) { + rethrowIfAborted(signal, error) + // Triple miss: every tier failed. Surface all three causes — the + // in-process dialog's reason is otherwise unrecoverable from the last + // PowerShell error alone. + throw new AggregateError( + [dialogError, pwshError, error], + 'native directory picker failed: the in-process dialog and both PowerShell hosts failed', + ) } - const result = await run('powershell.exe', ['-NoProfile', '-STA', '-Command', script], signal) - return outputPath(result.stdout) } if (platform === 'linux') { diff --git a/packages/host/directory-picker-native/src/win32-dialog-bindings.ts b/packages/host/directory-picker-native/src/win32-dialog-bindings.ts index a9b625812c..dc797a712e 100644 --- a/packages/host/directory-picker-native/src/win32-dialog-bindings.ts +++ b/packages/host/directory-picker-native/src/win32-dialog-bindings.ts @@ -23,6 +23,7 @@ interface Koffi { decode(value: unknown, offsetOrType: unknown, type?: unknown): unknown register(fn: (...args: unknown[]) => unknown, type: unknown): unknown unregister(callback: unknown): void + sizeof(type: string): number } const COINIT_APARTMENTTHREADED = 0x2 @@ -68,7 +69,11 @@ export async function loadWin32DialogBindings(): Promise { const user32 = koffi.load('user32.dll') const kernel32 = koffi.load('kernel32.dll') + // Vtable slots and out-pointers are pointer-width offsets: 8 on x64/arm64, + // 4 on ia32 — koffi reports the running process's width. + const pointerSize = koffi.sizeof('void *') const coInitializeEx = ole32.func('__stdcall', 'CoInitializeEx', 'int32', ['void *', 'uint32']) + const coUninitialize = ole32.func('__stdcall', 'CoUninitialize', 'void', []) const coCreateInstance = ole32.func('__stdcall', 'CoCreateInstance', 'int32', ['void *', 'void *', 'uint32', 'void *', 'void *']) const coTaskMemFree = ole32.func('__stdcall', 'CoTaskMemFree', 'void', ['void *']) const getCurrentThreadId = kernel32.func('__stdcall', 'GetCurrentThreadId', 'uint32', []) @@ -83,7 +88,7 @@ export async function loadWin32DialogBindings(): Promise { /** Bind vtable slot `slot` of COM object `self` to a caller through `proto`. */ const method = (self: unknown, slot: number, proto: unknown): (...args: unknown[]) => number => { const vtable = koffi.decode(self, 'void *') - const fn = koffi.decode(vtable, slot * 8, 'void *') + const fn = koffi.decode(vtable, slot * pointerSize, 'void *') return (...args: unknown[]) => koffi.call(fn, proto, self, ...args) as number } @@ -99,9 +104,12 @@ export async function loadWin32DialogBindings(): Promise { } }, coInitializeSta: () => coInitializeEx(null, COINIT_APARTMENTTHREADED) as number, + coUninitialize: () => { + coUninitialize() + }, currentThreadId: () => getCurrentThreadId() as number, createFolderDialog: (): Win32FolderDialog => { - const out = Buffer.alloc(8) + const out = Buffer.alloc(pointerSize) const created = coCreateInstance(CLSID_FILE_OPEN_DIALOG, null, CLSCTX_INPROC_SERVER, IID_IFILE_OPEN_DIALOG, out) as number if (created < 0) throw new Error(`CoCreateInstance(FileOpenDialog) failed: HRESULT 0x${(created >>> 0).toString(16)}`) const dialog = koffi.decode(out, 'void *') diff --git a/packages/host/directory-picker-native/src/win32-dialog-logic.ts b/packages/host/directory-picker-native/src/win32-dialog-logic.ts index cba0ca9f24..65be1149dc 100644 --- a/packages/host/directory-picker-native/src/win32-dialog-logic.ts +++ b/packages/host/directory-picker-native/src/win32-dialog-logic.ts @@ -59,6 +59,12 @@ export interface Win32DialogBindings { * @returns the call's HRESULT (`S_FALSE` re-entry is still a success). */ coInitializeSta(): number + /** + * `CoUninitialize` on the calling thread — COM requires one pairing call + * for every successful (including `S_FALSE`) `CoInitializeEx`, even on a + * thread that exits right after the conversation. + */ + coUninitialize(): void /** * `CoCreateInstance(CLSID_FileOpenDialog)`. * @returns the created dialog surface; throws when creation fails. @@ -100,18 +106,24 @@ export function runFolderDialog( ): string | null { bindings.setThreadDpiAwareness() check(bindings.coInitializeSta(), 'CoInitializeEx') - const dialog = bindings.createFolderDialog() + // From here the apartment is initialized (S_OK or S_FALSE) and must be + // uninitialized exactly once on every path. try { - check(dialog.setOptions(FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM | FOS_NOCHANGEDIR), 'SetOptions') - check(dialog.setTitle(title), 'SetTitle') - onShowing(bindings.currentThreadId()) - const shown = dialog.show() - if (shown === HRESULT_CANCELLED) return null - check(shown, 'Show') - const result = dialog.resultPath() - check(result.hr, 'GetResult') - return result.path as string + const dialog = bindings.createFolderDialog() + try { + check(dialog.setOptions(FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM | FOS_NOCHANGEDIR), 'SetOptions') + check(dialog.setTitle(title), 'SetTitle') + onShowing(bindings.currentThreadId()) + const shown = dialog.show() + if (shown === HRESULT_CANCELLED) return null + check(shown, 'Show') + const result = dialog.resultPath() + check(result.hr, 'GetResult') + return result.path as string + } finally { + dialog.release() + } } finally { - dialog.release() + bindings.coUninitialize() } } diff --git a/packages/host/directory-picker-native/src/win32-dialog-worker.ts b/packages/host/directory-picker-native/src/win32-dialog-worker.ts index 2e4ef64f6c..e978d305f3 100644 --- a/packages/host/directory-picker-native/src/win32-dialog-worker.ts +++ b/packages/host/directory-picker-native/src/win32-dialog-worker.ts @@ -28,7 +28,9 @@ const { title } = workerData as Win32DialogWorkerData void (async () => { try { const bindings = await loadWin32DialogBindings() - const path = runFolderDialog(bindings, title, (threadId) =>{ port.postMessage({ kind: 'showing', threadId } satisfies Win32DialogWorkerMessage) }) + const path = runFolderDialog(bindings, title, (threadId) => { + port.postMessage({ kind: 'showing', threadId } satisfies Win32DialogWorkerMessage) + }) port.postMessage({ kind: 'done', path } satisfies Win32DialogWorkerMessage) } catch (error: unknown) { const message = error instanceof Error ? (error.stack ?? error.message) : String(error) diff --git a/packages/host/directory-picker-native/src/win32-dialog.ts b/packages/host/directory-picker-native/src/win32-dialog.ts index 40b31e8d57..b97c3659fe 100644 --- a/packages/host/directory-picker-native/src/win32-dialog.ts +++ b/packages/host/directory-picker-native/src/win32-dialog.ts @@ -81,11 +81,20 @@ export async function pickWin32Directory( outcome() } + const postClose = (): void => { + // Before `showing` there is no window to close; the budget below still + // runs so a worker that never reports cannot dangle the pick. + if (dialogThreadId !== undefined) void closeWindows(dialogThreadId).catch(() => undefined) + } + + // Sole caller: the once-registered abort listener, so no re-entry guard. const serviceAbort = (): void => { let attempts = 0 // The `showing` notice precedes the blocking `Show`, so the very first // WM_CLOSE can race the window's creation; re-post until the worker - // reports back, then force-terminate as a last resort. + // reports back, then force-terminate as a last resort. The budget is + // unconditional — an abort before `showing` (worker hung in koffi or + // COM init) still ends in terminate instead of a dangling promise. closeTimer = setInterval(() => { attempts += 1 if (attempts > CLOSE_MAX_ATTEMPTS) { @@ -95,14 +104,13 @@ export async function pickWin32Directory( }) return } - void closeWindows(dialogThreadId as number).catch(() => undefined) + postClose() }, closeRetryMs) - void closeWindows(dialogThreadId as number).catch(() => undefined) + postClose() } const onAbort = (): void => { - if (dialogThreadId !== undefined) serviceAbort() - // Not shown yet: the `showing` handler below starts the service loop. + serviceAbort() } signal.addEventListener('abort', onAbort, { once: true }) @@ -110,7 +118,8 @@ export async function pickWin32Directory( switch (message.kind) { case 'showing': dialogThreadId = message.threadId - if (signal.aborted) serviceAbort() + // An abort that raced ahead of this notice now has a window to hit. + if (signal.aborted) postClose() return case 'done': settle(() => { @@ -119,10 +128,20 @@ export async function pickWin32Directory( }) return case 'error': - settle(() =>{ reject(new Error(`win32 folder dialog failed: ${message.message}`)) }) + settle(() => { + reject(new Error(`win32 folder dialog failed: ${message.message}`)) + }) } }) - worker.on('error', (error: Error) =>{ settle(() =>{ reject(error) }) }) - worker.on('exit', () =>{ settle(() =>{ reject(new Error('win32 folder dialog worker exited before reporting a result')) }) }) + worker.on('error', (error: Error) => { + settle(() => { + reject(error) + }) + }) + worker.on('exit', () => { + settle(() => { + reject(new Error('win32 folder dialog worker exited before reporting a result')) + }) + }) }) } diff --git a/packages/host/directory-picker-native/tests/native-picker.spec.ts b/packages/host/directory-picker-native/tests/native-picker.spec.ts index 7c51d552d4..64732e4e17 100644 --- a/packages/host/directory-picker-native/tests/native-picker.spec.ts +++ b/packages/host/directory-picker-native/tests/native-picker.spec.ts @@ -97,10 +97,16 @@ describe('native directory picker', () => { .mockResolvedValueOnce({ stdout: '', stderr: '' }) await expect(pickNativeDirectory(signal(), { platform: 'win32', run: cancelled, pickWin32Dialog: noDialog })).resolves.toBeNull() + // Triple miss: the surfaced AggregateError carries all three causes, + // including the otherwise-lost in-process dialog failure. const failed = vi.fn() .mockRejectedValueOnce(failure('ENOENT')) .mockRejectedValueOnce(failure(2)) - await expect(pickNativeDirectory(signal(), { platform: 'win32', run: failed, pickWin32Dialog: noDialog })).rejects.toThrow('command failed') + const tripleMiss = await pickNativeDirectory(signal(), { platform: 'win32', run: failed, pickWin32Dialog: noDialog }) + .then(() => { throw new Error('expected rejection') }, (error: unknown) => error as AggregateError) + expect(tripleMiss.message).toContain('the in-process dialog and both PowerShell hosts failed') + expect((tripleMiss.errors[0] as Error).message).toBe('dialog unavailable') + expect((tripleMiss.errors[2] as Error).message).toContain('command failed') }) it('wires the real Win32 dialog as the default tier', async () => { @@ -153,7 +159,9 @@ describe('native directory picker', () => { execFileMock.mockImplementationOnce((_command, _args, _options, callback) => { callback(commandError, 'partial output', 'failure details') }) - await expect(pickNativeDirectory(signal(), { platform: 'win32', pickWin32Dialog: noDialog })).rejects.toMatchObject({ + const surfaced = await pickNativeDirectory(signal(), { platform: 'win32', pickWin32Dialog: noDialog }) + .then(() => { throw new Error('expected rejection') }, (error: unknown) => error as AggregateError) + expect(surfaced.errors[2]).toMatchObject({ message: 'powershell failed', cause: commandError, code: 7, stdout: 'partial output', stderr: 'failure details', }) diff --git a/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts b/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts index d23b43011a..799c24a062 100644 --- a/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts +++ b/packages/host/directory-picker-native/tests/win32-dialog-bindings.spec.ts @@ -31,6 +31,7 @@ interface ComWorld { posted: { hwnd: unknown; message: number }[] registered: number unregistered: number + uninitialized: number } function comWorld(overrides: Partial = {}): ComWorld { @@ -39,7 +40,7 @@ function comWorld(overrides: Partial = {}): ComWorld { hasThreadDpi: true, enumThrows: false, path: 'C:\\选中\\directory', titles: [], options: [], dpiContexts: [], freed: [], released: [], posted: [], - registered: 0, unregistered: 0, + registered: 0, unregistered: 0, uninitialized: 0, ...overrides, } } @@ -85,6 +86,7 @@ function installFakeKoffi(world: ComWorld): void { func: (_convention: string, name: string, _result: string, _args: string[]) => { switch (name) { case 'CoInitializeEx': return () => world.coInitHr + case 'CoUninitialize': return () => { world.uninitialized += 1 } case 'CoCreateInstance': return (...args: unknown[]) => { if (world.coCreateHr < 0) return world.coCreateHr outBuffers.set(args[4], dialogPtr) @@ -109,6 +111,7 @@ function installFakeKoffi(world: ComWorld): void { }), proto: (declaration: string) => ({ declaration }), pointer: (type: unknown) => type, + sizeof: (type: string) => { void type; return 8 }, register: (fn: (hwnd: unknown, lparam: unknown) => number) => { world.registered += 1; return { fn } }, unregister: () => { world.unregistered += 1 }, decode: (value: unknown, offsetOrType: unknown): unknown => { @@ -153,6 +156,7 @@ describe('loadWin32DialogBindings over the fake COM world', () => { expect(showing).toHaveBeenCalledWith(31337) expect(world.freed).toHaveLength(1) expect(world.released).toEqual(['item', 'dialog']) + expect(world.uninitialized).toBe(1) }) it('maps dismissal, missing DPI support, and the S_FALSE CoInitializeEx', async () => { @@ -163,6 +167,7 @@ describe('loadWin32DialogBindings over the fake COM world', () => { expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBeNull() expect(world.dpiContexts).toEqual([]) expect(world.released).toEqual(['dialog']) + expect(world.uninitialized).toBe(1) }) it('surfaces creation and extraction failures as HRESULT errors', async () => { @@ -225,6 +230,7 @@ describe('the worker entry over a mocked thread boundary', () => { loadWin32DialogBindings: async () => ({ setThreadDpiAwareness: () => undefined, coInitializeSta: () => 0, + coUninitialize: () => undefined, currentThreadId: () => 11, createFolderDialog: () => ({ setOptions: () => 0, diff --git a/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts b/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts index c24a79ebc1..c214245de6 100644 --- a/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts +++ b/packages/host/directory-picker-native/tests/win32-dialog-logic.spec.ts @@ -16,6 +16,7 @@ interface FakeWorld { bindings: Win32DialogBindings dpi: ReturnType createDialog: ReturnType + uninitialize: ReturnType dialog: { setOptions: ReturnType setTitle: ReturnType @@ -36,21 +37,25 @@ function world(overrides: Partial = {}, coInit = 0): FakeWorl } const dpi = vi.fn() const createDialog = vi.fn(() => dialog) + const uninitialize = vi.fn() const bindings: Win32DialogBindings = { setThreadDpiAwareness: dpi, coInitializeSta: vi.fn(() => coInit), + coUninitialize: uninitialize, createFolderDialog: createDialog, currentThreadId: vi.fn(() => 4242), } - return { bindings, dpi, createDialog, dialog: dialog as FakeWorld['dialog'] } + return { bindings, dpi, createDialog, uninitialize, dialog: dialog as FakeWorld['dialog'] } } describe('runFolderDialog', () => { - it('sequences DPI, STA, options, title, show, and result extraction', () => { - const { bindings, dpi, dialog } = world() + it('sequences DPI, STA, options, title, show, result extraction, and apartment teardown', () => { + const { bindings, dpi, dialog, uninitialize } = world() const showing = vi.fn() expect(runFolderDialog(bindings, 'Pick', showing)).toBe('C:\\picked\\目录') expect(dpi).toHaveBeenCalledOnce() + expect(uninitialize).toHaveBeenCalledOnce() + expect(dialog.release.mock.invocationCallOrder[0]).toBeLessThan(uninitialize.mock.invocationCallOrder[0] as number) expect(dialog.setOptions).toHaveBeenCalledWith(FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM | FOS_NOCHANGEDIR) expect(dialog.setTitle).toHaveBeenCalledWith('Pick') expect(showing).toHaveBeenCalledWith(4242) @@ -58,11 +63,12 @@ describe('runFolderDialog', () => { expect(dialog.release).toHaveBeenCalledOnce() }) - it('maps the cancelled HRESULT to null and still releases the dialog', () => { - const { bindings, dialog } = world({ show: vi.fn(() => HRESULT_CANCELLED) }) + it('maps the cancelled HRESULT to null and still releases the dialog and apartment', () => { + const { bindings, dialog, uninitialize } = world({ show: vi.fn(() => HRESULT_CANCELLED) }) expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBeNull() expect(dialog.resultPath).not.toHaveBeenCalled() expect(dialog.release).toHaveBeenCalledOnce() + expect(uninitialize).toHaveBeenCalledOnce() }) it('accepts the S_FALSE re-entry HRESULT from CoInitializeEx', () => { @@ -70,10 +76,12 @@ describe('runFolderDialog', () => { expect(runFolderDialog(bindings, 'Pick', vi.fn())).toBe('C:\\picked\\目录') }) - it('throws on a failing CoInitializeEx without creating a dialog', () => { - const { bindings, createDialog } = world({}, E_FAIL) + it('throws on a failing CoInitializeEx without creating a dialog or uninitializing', () => { + const { bindings, createDialog, uninitialize } = world({}, E_FAIL) expect(() => runFolderDialog(bindings, 'Pick', vi.fn())).toThrow('CoInitializeEx failed: HRESULT 0x80004005') expect(createDialog).not.toHaveBeenCalled() + // A failed CoInitializeEx must NOT be paired with CoUninitialize. + expect(uninitialize).not.toHaveBeenCalled() }) it.each([ @@ -81,10 +89,11 @@ describe('runFolderDialog', () => { ['SetTitle', { setTitle: vi.fn(() => E_FAIL) }], ['Show', { show: vi.fn(() => E_FAIL) }], ['GetResult', { resultPath: vi.fn(() => ({ hr: E_FAIL })) }], - ] satisfies [string, Partial][])('releases the dialog when %s fails', (what, overrides) => { - const { bindings, dialog } = world(overrides) + ] satisfies [string, Partial][])('releases the dialog and apartment when %s fails', (what, overrides) => { + const { bindings, dialog, uninitialize } = world(overrides) expect(() => runFolderDialog(bindings, 'Pick', vi.fn())).toThrow(`${what} failed: HRESULT 0x80004005`) expect(dialog.release).toHaveBeenCalledOnce() + expect(uninitialize).toHaveBeenCalledOnce() void bindings }) }) diff --git a/packages/host/directory-picker-native/tests/win32-dialog.spec.ts b/packages/host/directory-picker-native/tests/win32-dialog.spec.ts index e3ea00d4a1..598386b1a7 100644 --- a/packages/host/directory-picker-native/tests/win32-dialog.spec.ts +++ b/packages/host/directory-picker-native/tests/win32-dialog.spec.ts @@ -94,7 +94,9 @@ describe('pickWin32Directory', () => { const picked = expect(pickWin32Directory(controller.signal, internals)).rejects.toThrow('native directory picker aborted') worker.post({ kind: 'showing', threadId: 99 }) controller.abort() - await vi.waitFor(() =>{ expect(close).toHaveBeenCalledWith(99) }) + await vi.waitFor(() => { + expect(close).toHaveBeenCalledWith(99) + }) worker.post({ kind: 'done', path: null }) await picked }) @@ -108,11 +110,25 @@ describe('pickWin32Directory', () => { controller.abort() expect(closeFailures).not.toHaveBeenCalled() worker.post({ kind: 'showing', threadId: 12 }) - await vi.waitFor(() =>{ expect(closeFailures.mock.calls.length).toBeGreaterThan(1) }) + await vi.waitFor(() => { + expect(closeFailures.mock.calls.length).toBeGreaterThan(1) + }) worker.post({ kind: 'done', path: null }) await picked }) + it('terminates a worker that never reports showing after an abort', async () => { + // The budget runs without a thread id (nothing to WM_CLOSE yet), so a + // worker hung before `showing` cannot dangle the pick. + const { worker, internals, close } = harness() + const controller = new AbortController() + const picked = expect(pickWin32Directory(controller.signal, internals)).rejects.toThrow('dialog unresponsive; worker terminated') + controller.abort() + await picked + expect(worker.terminate).toHaveBeenCalledOnce() + expect(close).not.toHaveBeenCalled() + }) + it('terminates an unresponsive worker after the close budget', async () => { const { worker, internals, close } = harness() const controller = new AbortController() @@ -134,7 +150,9 @@ describe('pickWin32Directory', () => { // and the abort service closes it (the same lever a disconnecting client pulls). it.skipIf(process.platform !== 'win32')('opens and abort-closes a real dialog', async () => { const controller = new AbortController() - setTimeout(() =>{ controller.abort() }, 400) + setTimeout(() => { + controller.abort() + }, 400) await expect(pickWin32Directory(controller.signal)).rejects.toThrow('native directory picker aborted') }, 30_000) })