mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix(code-index): prevent indexing memory leaks
This commit is contained in:
parent
c193f59819
commit
2eec91087f
10 changed files with 257 additions and 96 deletions
|
|
@ -7,6 +7,16 @@ import * as vscode from "vscode"
|
|||
|
||||
export const LOCK_TEXT_SYMBOL = "\u{1F512}"
|
||||
|
||||
export type RooIgnoreControllerOptions = {
|
||||
/**
|
||||
* When true (default), watches `.rooignore` for changes and reloads patterns.
|
||||
*
|
||||
* Set to false for short-lived usages (e.g., one-off scans) to avoid
|
||||
* creating long-lived VS Code file watchers.
|
||||
*/
|
||||
watch?: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Controls LLM access to files by enforcing ignore patterns.
|
||||
* Designed to be instantiated once in Cline.ts and passed to file manipulation services.
|
||||
|
|
@ -18,12 +28,14 @@ export class RooIgnoreController {
|
|||
private disposables: vscode.Disposable[] = []
|
||||
rooIgnoreContent: string | undefined
|
||||
|
||||
constructor(cwd: string) {
|
||||
constructor(cwd: string, options?: RooIgnoreControllerOptions) {
|
||||
this.cwd = cwd
|
||||
this.ignoreInstance = ignore()
|
||||
this.rooIgnoreContent = undefined
|
||||
// Set up file watcher for .rooignore
|
||||
this.setupFileWatcher()
|
||||
// Set up file watcher for .rooignore (optional)
|
||||
if (options?.watch !== false) {
|
||||
this.setupFileWatcher()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ export class CodeIndexManager {
|
|||
private _orchestrator: CodeIndexOrchestrator | undefined
|
||||
private _searchService: CodeIndexSearchService | undefined
|
||||
private _cacheManager: CacheManager | undefined
|
||||
private _rooIgnoreController: RooIgnoreController | undefined
|
||||
|
||||
// Flag to prevent race conditions during error recovery
|
||||
private _isRecoveringFromError = false
|
||||
|
|
@ -128,6 +129,8 @@ export class CodeIndexManager {
|
|||
if (this._orchestrator) {
|
||||
this._orchestrator.stopWatcher()
|
||||
}
|
||||
this._rooIgnoreController?.dispose()
|
||||
this._rooIgnoreController = undefined
|
||||
return { requiresRestart }
|
||||
}
|
||||
|
||||
|
|
@ -195,9 +198,6 @@ export class CodeIndexManager {
|
|||
* Stops the file watcher and potentially cleans up resources.
|
||||
*/
|
||||
public stopWatcher(): void {
|
||||
if (!this.isFeatureEnabled) {
|
||||
return
|
||||
}
|
||||
if (this._orchestrator) {
|
||||
this._orchestrator.stopWatcher()
|
||||
}
|
||||
|
|
@ -231,6 +231,8 @@ export class CodeIndexManager {
|
|||
// Log error but continue with recovery - clearing service instances is more important
|
||||
console.error("Failed to clear error state during recovery:", error)
|
||||
} finally {
|
||||
this._rooIgnoreController?.dispose()
|
||||
this._rooIgnoreController = undefined
|
||||
// Force re-initialization by clearing service instances
|
||||
// This ensures a clean slate even if state update failed
|
||||
this._configManager = undefined
|
||||
|
|
@ -250,6 +252,8 @@ export class CodeIndexManager {
|
|||
if (this._orchestrator) {
|
||||
this.stopWatcher()
|
||||
}
|
||||
this._rooIgnoreController?.dispose()
|
||||
this._rooIgnoreController = undefined
|
||||
this._stateManager.dispose()
|
||||
}
|
||||
|
||||
|
|
@ -293,6 +297,8 @@ export class CodeIndexManager {
|
|||
if (this._orchestrator) {
|
||||
this.stopWatcher()
|
||||
}
|
||||
this._rooIgnoreController?.dispose()
|
||||
this._rooIgnoreController = undefined
|
||||
// Clear existing services to ensure clean state
|
||||
this._orchestrator = undefined
|
||||
this._searchService = undefined
|
||||
|
|
@ -328,16 +334,22 @@ export class CodeIndexManager {
|
|||
})
|
||||
}
|
||||
|
||||
// Create RooIgnoreController instance
|
||||
// Create RooIgnoreController instance (long-lived while indexing is enabled)
|
||||
const rooIgnoreController = new RooIgnoreController(workspacePath)
|
||||
await rooIgnoreController.initialize()
|
||||
try {
|
||||
await rooIgnoreController.initialize()
|
||||
this._rooIgnoreController = rooIgnoreController
|
||||
} catch (error) {
|
||||
rooIgnoreController.dispose()
|
||||
throw error
|
||||
}
|
||||
|
||||
// (Re)Create shared service instances
|
||||
const { embedder, vectorStore, scanner, fileWatcher } = this._serviceFactory.createServices(
|
||||
this.context,
|
||||
this._cacheManager!,
|
||||
ignoreInstance,
|
||||
rooIgnoreController,
|
||||
this._rooIgnoreController,
|
||||
)
|
||||
|
||||
// Validate embedder configuration before proceeding
|
||||
|
|
@ -390,6 +402,8 @@ export class CodeIndexManager {
|
|||
if (this._orchestrator) {
|
||||
this._orchestrator.stopWatcher()
|
||||
}
|
||||
this._rooIgnoreController?.dispose()
|
||||
this._rooIgnoreController = undefined
|
||||
// Set state to indicate service is disabled
|
||||
this._stateManager.setSystemState("Standby", "Code indexing is disabled")
|
||||
return
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ vi.mock("../../cache-manager")
|
|||
vi.mock("../../../core/ignore/RooIgnoreController", () => ({
|
||||
RooIgnoreController: vi.fn().mockImplementation(() => ({
|
||||
validateAccess: vi.fn().mockReturnValue(true),
|
||||
dispose: vi.fn(),
|
||||
})),
|
||||
}))
|
||||
vi.mock("ignore")
|
||||
|
|
@ -284,5 +285,33 @@ describe("FileWatcher", () => {
|
|||
|
||||
expect(mockWatcher.dispose).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should dispose the internally-owned RooIgnoreController", async () => {
|
||||
await fileWatcher.initialize()
|
||||
const ignoreController = (fileWatcher as any).ignoreController as { dispose: () => void }
|
||||
const disposeSpy = vi.spyOn(ignoreController, "dispose")
|
||||
fileWatcher.dispose()
|
||||
|
||||
// The internally created controller should be disposed
|
||||
expect(disposeSpy).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it("should not dispose a provided RooIgnoreController", async () => {
|
||||
const providedIgnoreController = { validateAccess: vi.fn().mockReturnValue(true), dispose: vi.fn() }
|
||||
const customWatcher = new FileWatcher(
|
||||
"/mock/workspace",
|
||||
mockContext,
|
||||
mockCacheManager,
|
||||
mockEmbedder,
|
||||
mockVectorStore,
|
||||
mockIgnoreInstance,
|
||||
providedIgnoreController as any,
|
||||
)
|
||||
|
||||
await customWatcher.initialize()
|
||||
customWatcher.dispose()
|
||||
|
||||
expect(providedIgnoreController.dispose).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -65,6 +65,28 @@ describe("CodeParser", () => {
|
|||
vi.mocked(readFile).mockResolvedValue("// default test content")
|
||||
})
|
||||
|
||||
it("should delete tree-sitter parse trees to avoid retaining WASM memory", async () => {
|
||||
const deleteSpy = vi.fn()
|
||||
mockLanguageParser.js.parser.parse = vi.fn((content: string) => ({
|
||||
rootNode: {
|
||||
text: content,
|
||||
startPosition: { row: 0 },
|
||||
endPosition: { row: content.split("\n").length - 1 },
|
||||
children: [],
|
||||
type: "program",
|
||||
},
|
||||
delete: deleteSpy,
|
||||
}))
|
||||
mockLanguageParser.js.query.captures.mockReturnValue([])
|
||||
|
||||
await parser.parseFile("test.js", {
|
||||
content:
|
||||
"/* This is a long test content string that exceeds 100 characters to trigger parsing. It spans enough content for fallback chunking. */",
|
||||
})
|
||||
|
||||
expect(deleteSpy).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
describe("parseFile", () => {
|
||||
it("should return empty array for unsupported extensions", async () => {
|
||||
const result = await parser.parseFile("test.unsupported")
|
||||
|
|
|
|||
|
|
@ -57,7 +57,9 @@ vi.mock("vscode", () => ({
|
|||
},
|
||||
}))
|
||||
|
||||
vi.mock("../../../../core/ignore/RooIgnoreController")
|
||||
vi.mock("../../../../core/ignore/RooIgnoreController", () => ({
|
||||
RooIgnoreController: vi.fn(),
|
||||
}))
|
||||
vi.mock("ignore")
|
||||
|
||||
// Override the Jest-based mock with a vitest-compatible version
|
||||
|
|
@ -149,9 +151,27 @@ describe("DirectoryScanner", () => {
|
|||
// Get and mock the listFiles function
|
||||
const { listFiles } = await import("../../../glob/list-files")
|
||||
vi.mocked(listFiles).mockResolvedValue([["test/file1.js", "test/file2.js"], false])
|
||||
|
||||
// Default: ensure short-lived RooIgnoreController is disposed
|
||||
const { RooIgnoreController } = await import("../../../../core/ignore/RooIgnoreController")
|
||||
const MockedRooIgnoreController = vi.mocked(RooIgnoreController as unknown as any)
|
||||
MockedRooIgnoreController.mockImplementation(() => ({
|
||||
initialize: vi.fn().mockResolvedValue(undefined),
|
||||
filterPaths: vi.fn((paths: string[]) => paths),
|
||||
dispose: vi.fn(),
|
||||
}))
|
||||
})
|
||||
|
||||
describe("scanDirectory", () => {
|
||||
it("should dispose its short-lived RooIgnoreController", async () => {
|
||||
const { RooIgnoreController } = await import("../../../../core/ignore/RooIgnoreController")
|
||||
const MockedRooIgnoreController = vi.mocked(RooIgnoreController as unknown as any)
|
||||
|
||||
await scanner.scanDirectory("/test")
|
||||
const instance = MockedRooIgnoreController.mock.results[0]?.value
|
||||
expect(instance.dispose).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it("should skip files larger than MAX_FILE_SIZE_BYTES", async () => {
|
||||
const { listFiles } = await import("../../../glob/list-files")
|
||||
vi.mocked(listFiles).mockResolvedValue([["test/file1.js"], false])
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ export class FileWatcher implements IFileWatcher {
|
|||
private ignoreInstance?: Ignore
|
||||
private fileWatcher?: vscode.FileSystemWatcher
|
||||
private ignoreController: RooIgnoreController
|
||||
private readonly ownsIgnoreController: boolean
|
||||
private accumulatedEvents: Map<string, { uri: vscode.Uri; type: "create" | "change" | "delete" }> = new Map()
|
||||
private batchProcessDebounceTimer?: NodeJS.Timeout
|
||||
private readonly BATCH_DEBOUNCE_DELAY_MS = 500
|
||||
|
|
@ -82,6 +83,7 @@ export class FileWatcher implements IFileWatcher {
|
|||
ignoreController?: RooIgnoreController,
|
||||
batchSegmentThreshold?: number,
|
||||
) {
|
||||
this.ownsIgnoreController = ignoreController === undefined
|
||||
this.ignoreController = ignoreController || new RooIgnoreController(workspacePath)
|
||||
if (ignoreInstance) {
|
||||
this.ignoreInstance = ignoreInstance
|
||||
|
|
@ -127,6 +129,9 @@ export class FileWatcher implements IFileWatcher {
|
|||
if (this.batchProcessDebounceTimer) {
|
||||
clearTimeout(this.batchProcessDebounceTimer)
|
||||
}
|
||||
if (this.ownsIgnoreController) {
|
||||
this.ignoreController.dispose()
|
||||
}
|
||||
this._onDidStartBatchProcessing.dispose()
|
||||
this._onBatchProgressUpdate.dispose()
|
||||
this._onDidFinishBatchProcessing.dispose()
|
||||
|
|
|
|||
|
|
@ -150,83 +150,88 @@ export class CodeParser implements ICodeParser {
|
|||
}
|
||||
|
||||
const tree = language.parser.parse(content)
|
||||
try {
|
||||
// We don't need to get the query string from languageQueries since it's already loaded
|
||||
// in the language object
|
||||
const captures = tree ? language.query.captures(tree.rootNode) : []
|
||||
|
||||
// We don't need to get the query string from languageQueries since it's already loaded
|
||||
// in the language object
|
||||
const captures = tree ? language.query.captures(tree.rootNode) : []
|
||||
|
||||
// Check if captures are empty
|
||||
if (captures.length === 0) {
|
||||
if (content.length >= MIN_BLOCK_CHARS) {
|
||||
// Perform fallback chunking if content is large enough
|
||||
const blocks = this._performFallbackChunking(filePath, content, fileHash, seenSegmentHashes)
|
||||
return blocks
|
||||
} else {
|
||||
// Return empty if content is too small for fallback
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
const results: CodeBlock[] = []
|
||||
|
||||
// Process captures if not empty
|
||||
const queue: Node[] = Array.from(captures).map((capture) => capture.node)
|
||||
|
||||
while (queue.length > 0) {
|
||||
const currentNode = queue.shift()!
|
||||
// const lineSpan = currentNode.endPosition.row - currentNode.startPosition.row + 1 // Removed as per lint error
|
||||
|
||||
// Check if the node meets the minimum character requirement
|
||||
if (currentNode.text.length >= MIN_BLOCK_CHARS) {
|
||||
// If it also exceeds the maximum character limit, try to break it down
|
||||
if (currentNode.text.length > MAX_BLOCK_CHARS * MAX_CHARS_TOLERANCE_FACTOR) {
|
||||
if (currentNode.children.filter((child) => child !== null).length > 0) {
|
||||
// If it has children, process them instead
|
||||
queue.push(...currentNode.children.filter((child) => child !== null))
|
||||
} else {
|
||||
// If it's a leaf node, chunk it
|
||||
const chunkedBlocks = this._chunkLeafNodeByLines(
|
||||
currentNode,
|
||||
filePath,
|
||||
fileHash,
|
||||
seenSegmentHashes,
|
||||
)
|
||||
results.push(...chunkedBlocks)
|
||||
}
|
||||
// Check if captures are empty
|
||||
if (captures.length === 0) {
|
||||
if (content.length >= MIN_BLOCK_CHARS) {
|
||||
// Perform fallback chunking if content is large enough
|
||||
const blocks = this._performFallbackChunking(filePath, content, fileHash, seenSegmentHashes)
|
||||
return blocks
|
||||
} else {
|
||||
// Node meets min chars and is within max chars, create a block
|
||||
const identifier =
|
||||
currentNode.childForFieldName("name")?.text ||
|
||||
currentNode.children.find((c) => c?.type === "identifier")?.text ||
|
||||
null
|
||||
const type = currentNode.type
|
||||
const start_line = currentNode.startPosition.row + 1
|
||||
const end_line = currentNode.endPosition.row + 1
|
||||
const content = currentNode.text
|
||||
const contentPreview = content.slice(0, 100)
|
||||
const segmentHash = createHash("sha256")
|
||||
.update(`${filePath}-${start_line}-${end_line}-${content.length}-${contentPreview}`)
|
||||
.digest("hex")
|
||||
|
||||
if (!seenSegmentHashes.has(segmentHash)) {
|
||||
seenSegmentHashes.add(segmentHash)
|
||||
results.push({
|
||||
file_path: filePath,
|
||||
identifier,
|
||||
type,
|
||||
start_line,
|
||||
end_line,
|
||||
content,
|
||||
segmentHash,
|
||||
fileHash,
|
||||
})
|
||||
}
|
||||
// Return empty if content is too small for fallback
|
||||
return []
|
||||
}
|
||||
}
|
||||
// Nodes smaller than minBlockChars are ignored
|
||||
}
|
||||
|
||||
return results
|
||||
const results: CodeBlock[] = []
|
||||
|
||||
// Process captures if not empty
|
||||
const queue: Node[] = Array.from(captures).map((capture) => capture.node)
|
||||
|
||||
while (queue.length > 0) {
|
||||
const currentNode = queue.shift()!
|
||||
// const lineSpan = currentNode.endPosition.row - currentNode.startPosition.row + 1 // Removed as per lint error
|
||||
|
||||
// Check if the node meets the minimum character requirement
|
||||
if (currentNode.text.length >= MIN_BLOCK_CHARS) {
|
||||
// If it also exceeds the maximum character limit, try to break it down
|
||||
if (currentNode.text.length > MAX_BLOCK_CHARS * MAX_CHARS_TOLERANCE_FACTOR) {
|
||||
if (currentNode.children.filter((child) => child !== null).length > 0) {
|
||||
// If it has children, process them instead
|
||||
queue.push(...currentNode.children.filter((child) => child !== null))
|
||||
} else {
|
||||
// If it's a leaf node, chunk it
|
||||
const chunkedBlocks = this._chunkLeafNodeByLines(
|
||||
currentNode,
|
||||
filePath,
|
||||
fileHash,
|
||||
seenSegmentHashes,
|
||||
)
|
||||
results.push(...chunkedBlocks)
|
||||
}
|
||||
} else {
|
||||
// Node meets min chars and is within max chars, create a block
|
||||
const identifier =
|
||||
currentNode.childForFieldName("name")?.text ||
|
||||
currentNode.children.find((c) => c?.type === "identifier")?.text ||
|
||||
null
|
||||
const type = currentNode.type
|
||||
const start_line = currentNode.startPosition.row + 1
|
||||
const end_line = currentNode.endPosition.row + 1
|
||||
const content = currentNode.text
|
||||
const contentPreview = content.slice(0, 100)
|
||||
const segmentHash = createHash("sha256")
|
||||
.update(`${filePath}-${start_line}-${end_line}-${content.length}-${contentPreview}`)
|
||||
.digest("hex")
|
||||
|
||||
if (!seenSegmentHashes.has(segmentHash)) {
|
||||
seenSegmentHashes.add(segmentHash)
|
||||
results.push({
|
||||
file_path: filePath,
|
||||
identifier,
|
||||
type,
|
||||
start_line,
|
||||
end_line,
|
||||
content,
|
||||
segmentHash,
|
||||
fileHash,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
// Nodes smaller than minBlockChars are ignored
|
||||
}
|
||||
|
||||
return results
|
||||
} finally {
|
||||
// web-tree-sitter parse trees hold onto WASM memory; ensure we free it.
|
||||
// (In tests, parse() may return a plain object without delete().)
|
||||
;(tree as unknown as { delete?: () => void })?.delete?.()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -82,13 +82,19 @@ export class DirectoryScanner implements IDirectoryScanner {
|
|||
// Filter out directories (marked with trailing '/')
|
||||
const filePaths = allPaths.filter((p) => !p.endsWith("/"))
|
||||
|
||||
// Initialize RooIgnoreController if not provided
|
||||
const ignoreController = new RooIgnoreController(directoryPath)
|
||||
|
||||
await ignoreController.initialize()
|
||||
|
||||
// Filter paths using .rooignore
|
||||
const allowedPaths = ignoreController.filterPaths(filePaths)
|
||||
// Create a short-lived RooIgnoreController for filtering.
|
||||
// IMPORTANT: do not create a file watcher here (scan is short-lived).
|
||||
let allowedPaths: string[]
|
||||
{
|
||||
const ignoreController = new RooIgnoreController(directoryPath, { watch: false })
|
||||
try {
|
||||
await ignoreController.initialize()
|
||||
// Filter paths using .rooignore
|
||||
allowedPaths = ignoreController.filterPaths(filePaths)
|
||||
} finally {
|
||||
ignoreController.dispose()
|
||||
}
|
||||
}
|
||||
|
||||
// Filter by supported extensions, ignore patterns, and excluded directories
|
||||
const supportedPaths = allowedPaths.filter((filePath) => {
|
||||
|
|
|
|||
44
src/services/tree-sitter/__tests__/tree-disposal.spec.ts
Normal file
44
src/services/tree-sitter/__tests__/tree-disposal.spec.ts
Normal file
|
|
@ -0,0 +1,44 @@
|
|||
// npx vitest services/tree-sitter/__tests__/tree-disposal.spec.ts
|
||||
|
||||
vi.mock("fs/promises", () => ({
|
||||
readFile: vi.fn().mockResolvedValue("function x() {}"),
|
||||
}))
|
||||
|
||||
vi.mock("../../../utils/fs", () => ({
|
||||
fileExistsAtPath: vi.fn().mockResolvedValue(true),
|
||||
}))
|
||||
|
||||
vi.mock("../languageParser", () => ({
|
||||
loadRequiredLanguageParsers: vi.fn(),
|
||||
}))
|
||||
|
||||
import { parseSourceCodeDefinitionsForFile } from "../index"
|
||||
|
||||
describe("tree-sitter tree disposal", () => {
|
||||
it("should delete parse trees after extracting captures", async () => {
|
||||
const deleteSpy = vi.fn()
|
||||
const mockTree = {
|
||||
rootNode: {},
|
||||
delete: deleteSpy,
|
||||
}
|
||||
|
||||
const mockLanguageParsers = {
|
||||
ts: {
|
||||
parser: {
|
||||
parse: vi.fn().mockReturnValue(mockTree),
|
||||
},
|
||||
query: {
|
||||
captures: vi.fn().mockReturnValue([]),
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const { loadRequiredLanguageParsers } = await import("../languageParser")
|
||||
;(loadRequiredLanguageParsers as unknown as { mockResolvedValue: (v: unknown) => void }).mockResolvedValue(
|
||||
mockLanguageParsers,
|
||||
)
|
||||
|
||||
await parseSourceCodeDefinitionsForFile("/test/file.ts")
|
||||
expect(deleteSpy).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
@ -314,15 +314,19 @@ async function parseFile(
|
|||
try {
|
||||
// Parse the file content into an Abstract Syntax Tree (AST)
|
||||
const tree = parser.parse(fileContent)
|
||||
try {
|
||||
// Apply the query to the AST and get the captures
|
||||
const captures = tree ? query.captures(tree.rootNode) : []
|
||||
|
||||
// Apply the query to the AST and get the captures
|
||||
const captures = tree ? query.captures(tree.rootNode) : []
|
||||
// Split the file content into individual lines
|
||||
const lines = fileContent.split("\n")
|
||||
|
||||
// Split the file content into individual lines
|
||||
const lines = fileContent.split("\n")
|
||||
|
||||
// Process the captures
|
||||
return processCaptures(captures, lines, extLang)
|
||||
// Process the captures
|
||||
return processCaptures(captures, lines, extLang)
|
||||
} finally {
|
||||
// web-tree-sitter parse trees hold onto WASM memory; ensure we free it.
|
||||
;(tree as unknown as { delete?: () => void })?.delete?.()
|
||||
}
|
||||
} catch (error) {
|
||||
console.log(`Error parsing file: ${error}\n`)
|
||||
// Return null on parsing error to avoid showing error messages in the output
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue