fix: prevent stack overflow when listing files in deep directory structures #7589
Conversation
…event stack overflow - Replace recursive scanDirectory function with iterative implementation using a queue - Prevents "Maximum call stack size exceeded" error when indexing large codebases (200k+ blocks) - Maintains all existing functionality and directory filtering logic - All existing tests pass without modification Fixes #7588
dosubot
bot
added
size:S
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
src/services/glob/list-files.ts
Outdated
| const queue: QueueItem[] = [{ path: absolutePath, context: initialContext }] | ||
|
|
||
| while (queue.length > 0) { | ||
| const item = queue.shift() |
There was a problem hiding this comment.
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:
| const item = queue.shift() | |
| const item = queue.pop() |
You'd need to reverse the order when pushing to maintain FIFO behavior.
| context: ScanContext | ||
| } | ||
|
|
||
| const queue: QueueItem[] = [{ path: absolutePath, context: initialContext }] |
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| async function scanDirectory(currentPath: string, context: ScanContext): Promise<void> { | ||
| // Use iterative approach with a queue to avoid stack overflow on deep directory structures |
There was a problem hiding this comment.
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.
| } | ||
| await scanDirectory(fullDirPath, newContext) | ||
| // Add to queue instead of recursive call | ||
| queue.push({ path: fullDirPath, context: newContext }) |
There was a problem hiding this comment.
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.
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
…ue operations - Add limit hint parameter to stop scanning when enough directories are collected - Replace Array.shift() with index-based traversal for O(1) dequeue operations - Preserve first-level directory prioritization with limits
dosubot
bot
added
size:XL
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
src/services/glob/list-files.ts
Outdated
| let hasFirstLevelInQueue = false | ||
| for (let i = head; i < queue.length; i++) { | ||
| const depth = queue[i].path.split(path.sep).filter((p) => p.length > 0).length | ||
| if (depth === baseLevelDepth) { |
There was a problem hiding this comment.
Potential bug in the early termination check: the code compares queue item depth with baseLevelDepth, but first-level directories actually have a depth of baseLevelDepth + 1. This might cause premature termination. Consider updating the condition to check for (depth === baseLevelDepth + 1).
| if (depth === baseLevelDepth) { | |
| if (depth === baseLevelDepth + 1) { |
…flow fix - Keep queue-based iterative approach to prevent stack overflow on deep directory structures - Remove complex limitHint parameter and its associated logic - Add simple early termination when enough directories are collected (2x limit as buffer) - Simplify test cases to match the cleaner implementation - Maintain all core functionality with less code complexity
daniel-lxs
changed the title
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
Problem
When listing files in projects with very deep directory structures (e.g., node_modules), the recursive directory scanning could cause a stack overflow due to excessive recursion depth.
Solution
This PR fixes the stack overflow issue by converting the recursive directory scanning to an iterative queue-based approach.
Changes:
Key Benefits:
Testing
All existing tests pass. The implementation has been tested with deep directory structures to confirm the stack overflow is resolved.
Performance
The iterative approach with early termination ensures we don't scan more directories than necessary, maintaining good performance even in large codebases.