From 595097b5fcac8df2b2e3b908c2d7f385c522c269 Mon Sep 17 00:00:00 2001 From: Shani Singh Date: Fri, 7 Aug 2026 11:20:51 +0530 Subject: [PATCH] fix(clipboard): stream the temp root when sweeping expired staged files (#12917) cleanupExpiredRemoteClipboardFiles read the entire OS temp root with readdir({ withFileTypes: true }) and mapped every entry into Promise.all, so the prefix filter only ran after a promise already existed per entry. The sweep is fire-and-forget from registerClipboardIpcHandlers at startup, so a large %TEMP% froze the main process before the window came up. Stream the root with opendir, skip foreign entries before allocating, and cap in-flight removals at 8. --- .../window/clipboard-ipc-handlers.test.ts | 46 ++---- .../clipboard-remote-file-cleanup.test.ts | 153 ++++++++++++++++++ src/main/window/clipboard-remote-file-copy.ts | 57 ++++--- 3 files changed, 201 insertions(+), 55 deletions(-) create mode 100644 src/main/window/clipboard-remote-file-cleanup.test.ts diff --git a/src/main/window/clipboard-ipc-handlers.test.ts b/src/main/window/clipboard-ipc-handlers.test.ts index 406949ac6..b1da7e9e2 100644 --- a/src/main/window/clipboard-ipc-handlers.test.ts +++ b/src/main/window/clipboard-ipc-handlers.test.ts @@ -13,7 +13,7 @@ const { childStdinEndMock, resolveAuthorizedPathMock, fsMkdirMock, - fsReaddirMock, + fsOpendirMock, fsRmMock, fsWriteFileMock, fsOpenMock, @@ -46,7 +46,7 @@ const { }), resolveAuthorizedPathMock: vi.fn(), fsMkdirMock: vi.fn(), - fsReaddirMock: vi.fn(), + fsOpendirMock: vi.fn(), fsRmMock: vi.fn(), fsWriteFileMock: vi.fn(), fsOpenMock: vi.fn(), @@ -69,7 +69,7 @@ vi.mock('node:child_process', () => ({ vi.mock('node:fs/promises', () => ({ mkdir: fsMkdirMock, - readdir: fsReaddirMock, + opendir: fsOpendirMock, rm: fsRmMock, open: fsOpenMock, stat: fsStatMock, @@ -133,7 +133,6 @@ import { registerClipboardHandlers, setTrustedClipboardRendererWebContentsId } from './clipboard-ipc-handlers' -import { cleanupExpiredRemoteClipboardFiles } from './clipboard-remote-file-copy' function getRegisteredHandlers(): Map unknown> { const handlers = new Map unknown>() @@ -173,10 +172,6 @@ function trackPromiseSettled(promise: Promise): () => boolean { return () => settled } -function dirent(name: string, directory = true): { name: string; isDirectory: () => boolean } { - return { name, isDirectory: () => directory } -} - function shellIdListArray(childCount: number): Buffer { const value = Buffer.alloc(4 + 4 * (childCount + 1)) value.writeUInt32LE(childCount) @@ -194,8 +189,12 @@ describe('registerClipboardHandlers', () => { resolveAuthorizedPathMock.mockImplementation(async (path: string) => path) fsMkdirMock.mockReset() fsMkdirMock.mockResolvedValue(undefined) - fsReaddirMock.mockReset() - fsReaddirMock.mockResolvedValue([]) + fsOpendirMock.mockReset() + // Why: handler registration kicks off the expired-staging sweep; an empty temp root keeps it inert. + fsOpendirMock.mockImplementation(async () => ({ + async *[Symbol.asyncIterator]() {}, + close: vi.fn().mockResolvedValue(undefined) + })) fsRmMock.mockReset() fsRmMock.mockResolvedValue(undefined) fsWriteFileMock.mockReset() @@ -307,33 +306,6 @@ describe('registerClipboardHandlers', () => { } }) - it('sweeps expired remote clipboard staging directories', async () => { - const nowMs = 1760000000000 - fsReaddirMock.mockResolvedValue([ - dirent('orca-clipboard-file-expired'), - dirent('orca-clipboard-file-fresh'), - dirent('orca-clipboard-file-plain-file', false), - dirent('unrelated-temp') - ]) - fsStatMock.mockImplementation(async (targetPath: string) => { - if (targetPath.endsWith('expired')) { - return { mtimeMs: nowMs - 60 * 60 * 1000 - 1 } - } - if (targetPath.endsWith('fresh')) { - return { mtimeMs: nowMs - 1000 } - } - throw new Error(`unexpected stat: ${targetPath}`) - }) - - await cleanupExpiredRemoteClipboardFiles(nowMs) - - expect(fsRmMock).toHaveBeenCalledTimes(1) - expect(fsRmMock).toHaveBeenCalledWith(join('/tmp', 'orca-clipboard-file-expired'), { - recursive: true, - force: true - }) - }) - it('materializes remote files before writing them to the OS clipboard', async () => { const provider = { stat: vi.fn().mockResolvedValue({ size: 12, type: 'file', mtime: 123 }), diff --git a/src/main/window/clipboard-remote-file-cleanup.test.ts b/src/main/window/clipboard-remote-file-cleanup.test.ts new file mode 100644 index 000000000..56700cc02 --- /dev/null +++ b/src/main/window/clipboard-remote-file-cleanup.test.ts @@ -0,0 +1,153 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' + +const { opendirMock, rmMock, statMock } = vi.hoisted(() => ({ + opendirMock: vi.fn(), + rmMock: vi.fn(), + statMock: vi.fn() +})) + +vi.mock('node:fs/promises', () => ({ + mkdir: vi.fn(), + opendir: opendirMock, + rm: rmMock, + stat: statMock +})) +vi.mock('electron', () => ({ app: { getPath: () => '/tmp' } })) +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + requireSshFilesystemProvider: vi.fn() +})) +vi.mock('./clipboard-file-copy', () => ({ writeFileToClipboard: vi.fn() })) + +import { cleanupExpiredRemoteClipboardFiles } from './clipboard-remote-file-copy' + +const TTL_MS = 60 * 60 * 1000 +const NOW_MS = 1_760_000_000_000 + +function mockTempRoot(entries: Iterable<{ name: string; isDirectory: () => boolean }>): void { + opendirMock.mockResolvedValue({ + async *[Symbol.asyncIterator]() { + yield* entries + }, + close: vi.fn().mockResolvedValue(undefined) + }) +} + +function* orcaStagingDirs(count: number): Generator<{ name: string; isDirectory: () => boolean }> { + for (let index = 0; index < count; index += 1) { + yield { name: `orca-clipboard-file-expired-${index}`, isDirectory: () => true } + } +} + +function* unrelatedTempEntries( + count: number +): Generator<{ name: string; isDirectory: () => boolean }> { + for (let index = 0; index < count; index += 1) { + // Mirrors a real temp root: mostly foreign entries, both files and directories. + yield { name: `unrelated-${index}`, isDirectory: () => index % 2 === 0 } + } +} + +describe('cleanupExpiredRemoteClipboardFiles', () => { + beforeEach(() => { + vi.clearAllMocks() + rmMock.mockResolvedValue(undefined) + statMock.mockResolvedValue({ mtimeMs: NOW_MS - TTL_MS - 1 }) + }) + + it('streams all entries with at most eight cleanups in flight', async () => { + mockTempRoot(orcaStagingDirs(257)) + let active = 0 + let peak = 0 + statMock.mockImplementation(async () => { + active += 1 + peak = Math.max(peak, active) + await new Promise((resolve) => setImmediate(resolve)) + active -= 1 + return { mtimeMs: NOW_MS - TTL_MS - 1 } + }) + + await cleanupExpiredRemoteClipboardFiles(NOW_MS) + + expect(rmMock).toHaveBeenCalledTimes(257) + expect(peak).toBe(8) + }) + + it('does no per-entry work for foreign temp-root entries', async () => { + mockTempRoot(unrelatedTempEntries(200_000)) + + await cleanupExpiredRemoteClipboardFiles(NOW_MS) + + expect(statMock).not.toHaveBeenCalled() + expect(rmMock).not.toHaveBeenCalled() + }) + + it('acts on owned directories while the temp root is still being enumerated', async () => { + // Regression: the sweep materialized the whole temp root and built one promise + // per entry before doing any work, so a %TEMP% with ~1.2M unrelated entries + // froze the main process at startup (#12835). Streaming means an owned + // directory is swept before enumeration reaches the end. + let sweptDuringEnumeration = false + function* enumeration(): Generator<{ name: string; isDirectory: () => boolean }> { + yield { name: 'orca-clipboard-file-expired', isDirectory: () => true } + for (const entry of unrelatedTempEntries(1_000)) { + sweptDuringEnumeration ||= rmMock.mock.calls.length > 0 + yield entry + } + } + mockTempRoot(enumeration()) + + await cleanupExpiredRemoteClipboardFiles(NOW_MS) + + expect(sweptDuringEnumeration).toBe(true) + expect(rmMock).toHaveBeenCalledTimes(1) + }) + + it('removes only expired staging directories it owns', async () => { + function* interleaved(): Generator<{ name: string; isDirectory: () => boolean }> { + yield* unrelatedTempEntries(50_000) + yield { name: 'orca-clipboard-file-expired', isDirectory: () => true } + yield* unrelatedTempEntries(50_000) + yield { name: 'orca-clipboard-file-fresh', isDirectory: () => true } + yield { name: 'orca-clipboard-file-plain-file', isDirectory: () => false } + } + mockTempRoot(interleaved()) + statMock.mockImplementation(async (targetPath: string) => ({ + mtimeMs: targetPath.endsWith('expired') ? NOW_MS - TTL_MS - 1 : NOW_MS - 1000 + })) + + await cleanupExpiredRemoteClipboardFiles(NOW_MS) + + expect(statMock).toHaveBeenCalledTimes(2) + expect(rmMock).toHaveBeenCalledTimes(1) + expect(rmMock).toHaveBeenCalledWith(join('/tmp', 'orca-clipboard-file-expired'), { + recursive: true, + force: true + }) + }) + + it('closes the temp-root handle and still sweeps when enumeration fails midway', async () => { + const close = vi.fn().mockResolvedValue(undefined) + opendirMock.mockResolvedValue({ + async *[Symbol.asyncIterator]() { + yield { name: 'orca-clipboard-file-expired', isDirectory: () => true } + throw new Error('EIO') + }, + close + }) + + await expect(cleanupExpiredRemoteClipboardFiles(NOW_MS)).resolves.toBeUndefined() + + expect(rmMock).toHaveBeenCalledTimes(1) + expect(close).toHaveBeenCalledTimes(1) + }) + + it('returns quietly when the temp root cannot be opened', async () => { + opendirMock.mockRejectedValue(new Error('EACCES')) + + await expect(cleanupExpiredRemoteClipboardFiles(NOW_MS)).resolves.toBeUndefined() + + expect(statMock).not.toHaveBeenCalled() + expect(rmMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/window/clipboard-remote-file-copy.ts b/src/main/window/clipboard-remote-file-copy.ts index 052fce31e..2afc60cda 100644 --- a/src/main/window/clipboard-remote-file-copy.ts +++ b/src/main/window/clipboard-remote-file-copy.ts @@ -1,6 +1,6 @@ import { randomUUID } from 'node:crypto' -import type { Dirent } from 'node:fs' -import { mkdir, readdir, rm, stat } from 'node:fs/promises' +import type { Dir } from 'node:fs' +import { mkdir, opendir, rm, stat } from 'node:fs/promises' import { join } from 'node:path' import { app } from 'electron' @@ -17,6 +17,7 @@ type RemoteClipboardFileDeps = Omit const REMOTE_CLIPBOARD_FILE_TTL_MS = 60 * 60 * 1000 const REMOTE_CLIPBOARD_FILE_PREFIX = 'orca-clipboard-file-' +const REMOTE_CLIPBOARD_CLEANUP_CONCURRENCY = 8 const WINDOWS_RESERVED_LOCAL_BASENAME = /^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])(?:\..*)?$/i const LOCAL_FILENAME_REPLACEMENT_CHARS = new Set(['<', '>', ':', '"', '/', '\\', '|', '?', '*']) @@ -79,32 +80,52 @@ export async function writeRemoteFileToClipboard({ } } +// Why: the OS temp root is shared with every other program and routinely holds +// millions of unrelated entries on long-lived machines, so this startup sweep +// streams it and only ever retains work for entries it actually owns. export async function cleanupExpiredRemoteClipboardFiles(nowMs = Date.now()): Promise { const tempRoot = app.getPath('temp') - let entries: Dirent[] + let tempRootDir: Dir try { - entries = await readdir(tempRoot, { withFileTypes: true }) + tempRootDir = await opendir(tempRoot) } catch { return } - await Promise.all( - entries.map(async (entry) => { + const pending = new Set>() + try { + for await (const entry of tempRootDir) { if (!entry.isDirectory() || !entry.name.startsWith(REMOTE_CLIPBOARD_FILE_PREFIX)) { - return + continue } - const tempDir = join(tempRoot, entry.name) - try { - const tempStats = await stat(tempDir) - if (nowMs - tempStats.mtimeMs < REMOTE_CLIPBOARD_FILE_TTL_MS) { - return - } - await rm(tempDir, { recursive: true, force: true }) - } catch { - // Why: stale staged SSH files should not make startup cleanup noisy. + const cleanup = cleanupExpiredRemoteClipboardDirectory(join(tempRoot, entry.name), nowMs) + pending.add(cleanup) + void cleanup.finally(() => pending.delete(cleanup)) + if (pending.size >= REMOTE_CLIPBOARD_CLEANUP_CONCURRENCY) { + await Promise.race(pending) } - }) - ) + } + } catch { + // Why: a partial best-effort sweep beats failing startup on a temp-root read error. + } finally { + // Why: exhausting the iterator already closes the handle; closing again is harmless. + await tempRootDir.close().catch(() => undefined) + } + await Promise.all(pending) +} + +async function cleanupExpiredRemoteClipboardDirectory( + tempDir: string, + nowMs: number +): Promise { + try { + const tempStats = await stat(tempDir) + if (nowMs - tempStats.mtimeMs >= REMOTE_CLIPBOARD_FILE_TTL_MS) { + await rm(tempDir, { recursive: true, force: true }) + } + } catch { + // Why: stale staged SSH files should not make startup cleanup noisy. + } } function sanitizeLocalClipboardFilename(remoteBasename: string): string {