Skip to content

fix: prevent stack overflow when listing files in deep directory structures #7589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

fix: prevent stack overflow when listing files in deep directory structures #7589

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbcb52c
fix: convert recursive directory scanning to iterative approach to pr…
roomote Sep 2, 2025
2293eb8
perf: optimize directory scanning with early termination and O(1) que…
daniel-lxs Sep 4, 2025
3963e38
refactor: simplify list-files implementation while keeping stack over…
daniel-lxs Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/services/glob/list-files.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,20 @@ async function listFilteredDirectories(
ignoreInstance,
}

async function scanDirectory(currentPath: string, context: ScanContext): Promise<void> {
// Use iterative approach with a queue to avoid stack overflow on deep directory structures
Copy link
Contributor Author

@roomote roomote bot Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add more detailed documentation here explaining why we switched from recursion to iteration? Future maintainers (including future me) would benefit from understanding the context of this architectural decision.

interface QueueItem {
path: string
context: ScanContext
}

const queue: QueueItem[] = [{ path: absolutePath, context: initialContext }]
Copy link
Contributor Author

@roomote roomote bot Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this iterative approach successfully prevents stack overflow, could we consider adding a queue size limit or batch processing for extremely large directory structures? The queue could potentially consume significant memory if it grows unbounded.


while (queue.length > 0) {
const item = queue.shift()
Copy link
Contributor Author

@roomote roomote bot Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is using Array.shift() intentional here? It has O(n) time complexity which could impact performance for very large directory structures. Consider using a proper queue data structure or reversing the array and using pop() instead:

Suggested change
const item = queue.shift()
const item = queue.pop()

You'd need to reverse the order when pushing to maintain FIFO behavior.

if (!item) continue

const { path: currentPath, context } = item

try {
// List all entries in the current directory
const entries = await fs.promises.readdir(currentPath, { withFileTypes: true })
Expand Down Expand Up @@ -461,7 +474,8 @@ async function listFilteredDirectories(
isTargetDir: false,
insideExplicitHiddenTarget: newInsideExplicitHiddenTarget,
}
await scanDirectory(fullDirPath, newContext)
// Add to queue instead of recursive call
queue.push({ path: fullDirPath, context: newContext })
Copy link
Contributor Author

@roomote roomote bot Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider adding a specific test case that simulates a very deep directory structure (like 10,000+ levels) to ensure this iterative approach handles extreme edge cases properly? The existing tests pass, but they might not stress-test the depth that caused the original issue.

}
}
}
Expand All @@ -471,9 +485,6 @@ async function listFilteredDirectories(
}
}

// Start scanning from the root directory
await scanDirectory(absolutePath, initialContext)

return directories
}

Expand Down
Loading