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.
This commit is contained in:
parent
2b42de1f52
commit
595097b5fc
|
|
@ -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<string, (...args: unknown[]) => unknown> {
|
||||
const handlers = new Map<string, (...args: unknown[]) => unknown>()
|
||||
|
|
@ -173,10 +172,6 @@ function trackPromiseSettled(promise: Promise<unknown>): () => 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 }),
|
||||
|
|
|
|||
|
|
@ -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<void>((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()
|
||||
})
|
||||
})
|
||||
|
|
@ -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<ClipboardFileDeps, 'resolveFilePath'>
|
|||
|
||||
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<void> {
|
||||
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<Promise<void>>()
|
||||
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<void> {
|
||||
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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue