From 8b0907de3e3d0956de5d61ea6a7b8fc41471ff99 Mon Sep 17 00:00:00 2001 From: superphosphate <193793899+superphosphate@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:14:32 +0800 Subject: [PATCH 1/4] fix: prevent out of memory crash on cyclic symbolic links fast-glob follows symbolic links while walking the workspace, so a cyclic symbolic link made the language server read the same directory over and over again until the process ran out of memory. Read every directory by its real path only once, which breaks cycles while keeping symbolic links working. --- server/src/util/__tests__/fs.test.ts | 93 ++++++++++++++++++++++++++++ server/src/util/fs.ts | 59 ++++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 server/src/util/__tests__/fs.test.ts diff --git a/server/src/util/__tests__/fs.test.ts b/server/src/util/__tests__/fs.test.ts new file mode 100644 index 000000000..acc3cff10 --- /dev/null +++ b/server/src/util/__tests__/fs.test.ts @@ -0,0 +1,93 @@ +import * as fs from 'node:fs' +import * as os from 'node:os' +import * as path from 'node:path' + +import { getFilePaths } from '../fs' + +const symlinkType = process.platform === 'win32' ? 'junction' : 'dir' + +const relativePaths = (filePaths: string[], rootPath: string): string[] => + filePaths.map((filePath) => path.relative(rootPath, filePath).split(path.sep).join('/')) + +describe('getFilePaths', () => { + let rootPath: string + + beforeEach(() => { + rootPath = fs.mkdtempSync(path.join(os.tmpdir(), 'bash-language-server-fs-')) + }) + + afterEach(() => { + fs.rmSync(rootPath, { recursive: true, force: true }) + }) + + it('returns the paths of files matching the glob pattern', async () => { + fs.writeFileSync(path.join(rootPath, 'script.sh'), '') + fs.mkdirSync(path.join(rootPath, 'nested')) + fs.writeFileSync(path.join(rootPath, 'nested', 'nested.sh'), '') + + const filePaths = await getFilePaths({ + globPattern: '**/*.sh', + rootPath, + maxItems: 100, + }) + + expect(relativePaths(filePaths, rootPath).sort()).toEqual([ + 'nested/nested.sh', + 'script.sh', + ]) + }) + + it('follows symbolic links to directories', async () => { + const targetPath = fs.mkdtempSync( + path.join(os.tmpdir(), 'bash-language-server-fs-target-'), + ) + + try { + fs.writeFileSync(path.join(targetPath, 'linked.sh'), '') + fs.symlinkSync(targetPath, path.join(rootPath, 'link'), symlinkType) + + const filePaths = await getFilePaths({ + globPattern: '**/*.sh', + rootPath, + maxItems: 100, + }) + + expect(relativePaths(filePaths, rootPath)).toEqual(['link/linked.sh']) + } finally { + fs.rmSync(targetPath, { recursive: true, force: true }) + } + }) + + it('does not follow cyclic symbolic links', async () => { + fs.writeFileSync(path.join(rootPath, 'script.sh'), '') + + const loopPath = path.join(rootPath, 'loop') + fs.symlinkSync(rootPath, loopPath, symlinkType) + + try { + const filePaths = await getFilePaths({ + globPattern: '**/*.sh', + rootPath, + maxItems: 100, + }) + + expect(relativePaths(filePaths, rootPath)).toEqual(['script.sh']) + } finally { + fs.unlinkSync(loopPath) + } + }) + + it('stops after the maximum number of files', async () => { + for (let i = 0; i < 10; i++) { + fs.writeFileSync(path.join(rootPath, `script-${i}.sh`), '') + } + + const filePaths = await getFilePaths({ + globPattern: '**/*.sh', + rootPath, + maxItems: 3, + }) + + expect(filePaths).toHaveLength(3) + }) +}) diff --git a/server/src/util/fs.ts b/server/src/util/fs.ts index 3c822f9c7..31d545e27 100644 --- a/server/src/util/fs.ts +++ b/server/src/util/fs.ts @@ -1,3 +1,4 @@ +import * as fs from 'node:fs' import * as os from 'node:os' import { fileURLToPath } from 'node:url' @@ -11,6 +12,63 @@ export function untildify(pathWithTilde: string): string { : pathWithTilde } +/** + * Create a file system adapter for `fast-glob` that reads a directory only + * once, even when it is reachable through multiple (symbolic) links. + * + * `fast-glob` follows symbolic links, so a cyclic symbolic link makes it walk + * the same directory over and over again until the process runs out of memory. + * Reading every directory by its real path only once breaks such cycles while + * keeping symbolic links working. + */ +function createCycleSafeFileSystemAdapter( + readRealPaths: Set, +): Partial { + const isFirstReadOf = (realPath: string): boolean => { + if (readRealPaths.has(realPath)) { + return false + } + + readRealPaths.add(realPath) + + return true + } + + return { + readdir: (directoryPath: string, optionsOrCallback: any, callback?: any) => { + const options = + typeof optionsOrCallback === 'function' ? undefined : optionsOrCallback + const done = typeof optionsOrCallback === 'function' ? optionsOrCallback : callback + + fs.realpath(directoryPath, (realPathError, realPath) => { + if (realPathError == null && !isFirstReadOf(realPath)) { + done(null, []) + return + } + + if (options == null) { + fs.readdir(directoryPath, done) + } else { + fs.readdir(directoryPath, options, done) + } + }) + }, + readdirSync: (directoryPath: string, options?: any) => { + try { + if (!isFirstReadOf(fs.realpathSync(directoryPath))) { + return [] + } + } catch { + // fall through and let `readdirSync` report the error + } + + return options == null + ? fs.readdirSync(directoryPath) + : fs.readdirSync(directoryPath, options) + }, + } as Partial +} + export async function getFilePaths({ globPattern, rootPath, @@ -29,6 +87,7 @@ export async function getFilePaths({ onlyFiles: true, cwd: rootPath, followSymbolicLinks: true, + fs: createCycleSafeFileSystemAdapter(new Set()), suppressErrors: true, }) From e6e9f64354852e755578a4f8e4a9f2efd5c9baee Mon Sep 17 00:00:00 2001 From: superphosphate <193793899+superphosphate@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:05:46 +0800 Subject: [PATCH 2/4] fix: only skip directories that link back to an ancestor Skipping every already-read real path also dropped matches for the same directory reachable through several distinct symbolic links. Compare the real path with the real paths of the ancestor directories instead, so cycles are still broken while `{a,b}/**` matches both prefixes. --- server/src/util/__tests__/fs.test.ts | 45 ++++++++++++++++++++++++++++ server/src/util/fs.ts | 42 ++++++++++++++++++-------- 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/server/src/util/__tests__/fs.test.ts b/server/src/util/__tests__/fs.test.ts index acc3cff10..62dcb04e0 100644 --- a/server/src/util/__tests__/fs.test.ts +++ b/server/src/util/__tests__/fs.test.ts @@ -58,6 +58,31 @@ describe('getFilePaths', () => { } }) + it('follows several symbolic links to the same directory', async () => { + const targetPath = fs.mkdtempSync( + path.join(os.tmpdir(), 'bash-language-server-fs-target-'), + ) + + try { + fs.writeFileSync(path.join(targetPath, 'shared.sh'), '') + fs.symlinkSync(targetPath, path.join(rootPath, 'a'), symlinkType) + fs.symlinkSync(targetPath, path.join(rootPath, 'b'), symlinkType) + + const filePaths = await getFilePaths({ + globPattern: '{a,b}/**/*.sh', + rootPath, + maxItems: 100, + }) + + expect(relativePaths(filePaths, rootPath).sort()).toEqual([ + 'a/shared.sh', + 'b/shared.sh', + ]) + } finally { + fs.rmSync(targetPath, { recursive: true, force: true }) + } + }) + it('does not follow cyclic symbolic links', async () => { fs.writeFileSync(path.join(rootPath, 'script.sh'), '') @@ -77,6 +102,26 @@ describe('getFilePaths', () => { } }) + it('does not follow cyclic symbolic links to an ancestor', async () => { + fs.mkdirSync(path.join(rootPath, 'nested')) + fs.writeFileSync(path.join(rootPath, 'nested', 'nested.sh'), '') + + const loopPath = path.join(rootPath, 'nested', 'loop') + fs.symlinkSync(rootPath, loopPath, symlinkType) + + try { + const filePaths = await getFilePaths({ + globPattern: '**/*.sh', + rootPath, + maxItems: 100, + }) + + expect(relativePaths(filePaths, rootPath)).toEqual(['nested/nested.sh']) + } finally { + fs.unlinkSync(loopPath) + } + }) + it('stops after the maximum number of files', async () => { for (let i = 0; i < 10; i++) { fs.writeFileSync(path.join(rootPath, `script-${i}.sh`), '') diff --git a/server/src/util/fs.ts b/server/src/util/fs.ts index 31d545e27..8dcf939c4 100644 --- a/server/src/util/fs.ts +++ b/server/src/util/fs.ts @@ -1,5 +1,6 @@ import * as fs from 'node:fs' import * as os from 'node:os' +import * as path from 'node:path' import { fileURLToPath } from 'node:url' import * as fastGlob from 'fast-glob' @@ -13,25 +14,40 @@ export function untildify(pathWithTilde: string): string { } /** - * Create a file system adapter for `fast-glob` that reads a directory only - * once, even when it is reachable through multiple (symbolic) links. + * Create a file system adapter for `fast-glob` that stops walking a directory + * when it links back to one of its own ancestors. * * `fast-glob` follows symbolic links, so a cyclic symbolic link makes it walk * the same directory over and over again until the process runs out of memory. - * Reading every directory by its real path only once breaks such cycles while - * keeping symbolic links working. + * A directory is only skipped when its real path is the real path of one of + * its ancestors, so symbolic links in general, including several links to the + * same directory, keep working. */ function createCycleSafeFileSystemAdapter( - readRealPaths: Set, + realPaths: Map, ): Partial { - const isFirstReadOf = (realPath: string): boolean => { - if (readRealPaths.has(realPath)) { - return false + const isAncestorCycle = (directoryPath: string, realPath: string): boolean => { + let currentPath = directoryPath + let parentPath = path.dirname(currentPath) + + while (parentPath !== currentPath) { + if (realPaths.get(parentPath) === realPath) { + return true + } + + currentPath = parentPath + parentPath = path.dirname(currentPath) } - readRealPaths.add(realPath) + return false + } + + const readDirectory = (directoryPath: string, realPath: string): boolean => { + const isCycle = isAncestorCycle(directoryPath, realPath) + + realPaths.set(directoryPath, realPath) - return true + return !isCycle } return { @@ -41,7 +57,7 @@ function createCycleSafeFileSystemAdapter( const done = typeof optionsOrCallback === 'function' ? optionsOrCallback : callback fs.realpath(directoryPath, (realPathError, realPath) => { - if (realPathError == null && !isFirstReadOf(realPath)) { + if (realPathError == null && !readDirectory(directoryPath, realPath)) { done(null, []) return } @@ -55,7 +71,7 @@ function createCycleSafeFileSystemAdapter( }, readdirSync: (directoryPath: string, options?: any) => { try { - if (!isFirstReadOf(fs.realpathSync(directoryPath))) { + if (!readDirectory(directoryPath, fs.realpathSync(directoryPath))) { return [] } } catch { @@ -87,7 +103,7 @@ export async function getFilePaths({ onlyFiles: true, cwd: rootPath, followSymbolicLinks: true, - fs: createCycleSafeFileSystemAdapter(new Set()), + fs: createCycleSafeFileSystemAdapter(new Map()), suppressErrors: true, }) From ade07f6ca7721aedb2a702972e3689125f07826f Mon Sep 17 00:00:00 2001 From: superphosphate <193793899+superphosphate@users.noreply.github.com> Date: Thu, 10 Sep 2026 14:31:37 +0800 Subject: [PATCH 3/4] docs: reference the fast-glob symlink-cycle limitation Point at mrmlnc/fast-glob#74 in the adapter doc so the reason for the cycle guard (and why the upstream workarounds were not used) is recorded in the code. --- server/src/util/fs.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/util/fs.ts b/server/src/util/fs.ts index 8dcf939c4..5d5619134 100644 --- a/server/src/util/fs.ts +++ b/server/src/util/fs.ts @@ -17,8 +17,12 @@ export function untildify(pathWithTilde: string): string { * Create a file system adapter for `fast-glob` that stops walking a directory * when it links back to one of its own ancestors. * - * `fast-glob` follows symbolic links, so a cyclic symbolic link makes it walk - * the same directory over and over again until the process runs out of memory. + * `fast-glob` follows symbolic links by default and, like `node-glob`, walks a + * cyclic symbolic link forever until the process runs out of memory. Upstream + * tracks this as a known limitation with no fix, and the suggested workarounds + * (`followSymbolicLinks: false` or a `deep` limit) either drop symlink support + * or truncate deep trees — see https://github.com/mrmlnc/fast-glob/issues/74. + * * A directory is only skipped when its real path is the real path of one of * its ancestors, so symbolic links in general, including several links to the * same directory, keep working. From dd86458d639a012d1c6fdcd3e18c8fdac9c1f7f2 Mon Sep 17 00:00:00 2001 From: superphosphate <193793899+superphosphate@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:23:14 +0800 Subject: [PATCH 4/4] perf: only check symbolic links for cycles Calling `realpath` for every directory made the workspace scan several times slower on repositories with a populated `node_modules`, which pushed the workspace-wide rename test over its timeout. The walk can only descend, so a cycle has to be entered through a symbolic link: check only those, with `realpathSync.native` and memoized ancestor real paths. Plain directories now cost a single set lookup. --- server/src/util/fs.ts | 131 ++++++++++++++++++++++++++++++------------ 1 file changed, 94 insertions(+), 37 deletions(-) diff --git a/server/src/util/fs.ts b/server/src/util/fs.ts index 5d5619134..4ee742f14 100644 --- a/server/src/util/fs.ts +++ b/server/src/util/fs.ts @@ -23,35 +23,83 @@ export function untildify(pathWithTilde: string): string { * (`followSymbolicLinks: false` or a `deep` limit) either drop symlink support * or truncate deep trees — see https://github.com/mrmlnc/fast-glob/issues/74. * - * A directory is only skipped when its real path is the real path of one of - * its ancestors, so symbolic links in general, including several links to the - * same directory, keep working. + * Only the directories that are symbolic links are checked: the walk can only + * descend, so every cycle has to be entered through a symbolic link, and a + * plain directory can never link back to an ancestor. Directories are listed + * with `withFileTypes`, which is what `fast-glob` does on supported Node + * versions, so the common case does not pay for a `realpath` call at all. */ -function createCycleSafeFileSystemAdapter( - realPaths: Map, -): Partial { - const isAncestorCycle = (directoryPath: string, realPath: string): boolean => { - let currentPath = directoryPath - let parentPath = path.dirname(currentPath) - - while (parentPath !== currentPath) { - if (realPaths.get(parentPath) === realPath) { +function createCycleSafeFileSystemAdapter(): Partial { + // Symbolic links to directories, by normalized path. + const symlinkedDirectories = new Set() + // Real paths of the directories that had to be checked, by normalized path. + const realPaths = new Map() + // `readdir` without `withFileTypes` cannot report symbolic links, and from + // that point on every directory has to be checked. + let canDetectSymlinks = true + + const realPathOf = (directoryPath: string): string => { + let realPath = realPaths.get(directoryPath) + + if (realPath === undefined) { + try { + // The native implementation resolves paths in a single system call, + // which is considerably cheaper than the JavaScript fallback. + realPath = fs.realpathSync.native(directoryPath) + } catch { + realPath = directoryPath + } + + realPaths.set(directoryPath, realPath) + } + + return realPath + } + + const linksBackToAncestor = (directoryPath: string): boolean => { + const realPath = realPathOf(directoryPath) + let parentPath = path.dirname(directoryPath) + + while (parentPath !== directoryPath) { + if (realPathOf(parentPath) === realPath) { return true } - currentPath = parentPath - parentPath = path.dirname(currentPath) + directoryPath = parentPath + parentPath = path.dirname(directoryPath) } return false } - const readDirectory = (directoryPath: string, realPath: string): boolean => { - const isCycle = isAncestorCycle(directoryPath, realPath) + const isCycle = (directoryPath: string): boolean => { + const normalizedPath = path.normalize(directoryPath) + + if (canDetectSymlinks && !symlinkedDirectories.has(normalizedPath)) { + return false + } + + return linksBackToAncestor(normalizedPath) + } + + const recordEntries = (directoryPath: string, entries: unknown): void => { + if (!Array.isArray(entries)) { + canDetectSymlinks = false + return + } + + for (const entry of entries) { + if (typeof entry === 'string') { + canDetectSymlinks = false + return + } - realPaths.set(directoryPath, realPath) + const dirent = entry as fs.Dirent - return !isCycle + if (typeof dirent?.isSymbolicLink === 'function' && dirent.isSymbolicLink()) { + symlinkedDirectories.add(path.normalize(path.join(directoryPath, dirent.name))) + } + } } return { @@ -60,31 +108,40 @@ function createCycleSafeFileSystemAdapter( typeof optionsOrCallback === 'function' ? undefined : optionsOrCallback const done = typeof optionsOrCallback === 'function' ? optionsOrCallback : callback - fs.realpath(directoryPath, (realPathError, realPath) => { - if (realPathError == null && !readDirectory(directoryPath, realPath)) { - done(null, []) + if (isCycle(directoryPath)) { + done(null, []) + return + } + + const onRead = (error: NodeJS.ErrnoException | null, entries: unknown) => { + if (error != null) { + done(error) return } - if (options == null) { - fs.readdir(directoryPath, done) - } else { - fs.readdir(directoryPath, options, done) - } - }) + recordEntries(directoryPath, entries) + done(null, entries) + } + + if (options == null) { + fs.readdir(directoryPath, onRead) + } else { + fs.readdir(directoryPath, options, onRead) + } }, readdirSync: (directoryPath: string, options?: any) => { - try { - if (!readDirectory(directoryPath, fs.realpathSync(directoryPath))) { - return [] - } - } catch { - // fall through and let `readdirSync` report the error + if (isCycle(directoryPath)) { + return [] } - return options == null - ? fs.readdirSync(directoryPath) - : fs.readdirSync(directoryPath, options) + const entries = + options == null + ? fs.readdirSync(directoryPath) + : fs.readdirSync(directoryPath, options) + + recordEntries(directoryPath, entries) + + return entries }, } as Partial } @@ -107,7 +164,7 @@ export async function getFilePaths({ onlyFiles: true, cwd: rootPath, followSymbolicLinks: true, - fs: createCycleSafeFileSystemAdapter(new Map()), + fs: createCycleSafeFileSystemAdapter(), suppressErrors: true, })