-
Notifications
You must be signed in to change notification settings - Fork 17
fix: free leaked WASM trees in native engine typeMap backfill #534
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.
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
Changes from 5 commits
50ec4a0
1468ef1
7491363
73d755f
4b4545c
a5a66ee
c0c0c9b
83bbd2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -130,12 +130,19 @@ export const DEFAULTS = { | |||||||||||||
| }, | ||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| // Per-cwd config cache — avoids re-reading the config file on every query call. | ||||||||||||||
| // The config file rarely changes within a single process lifetime. | ||||||||||||||
| const _configCache = new Map(); | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Load project configuration from a .codegraphrc.json or similar file. | ||||||||||||||
| * Returns merged config with defaults. | ||||||||||||||
| * Returns merged config with defaults. Results are cached per cwd. | ||||||||||||||
| */ | ||||||||||||||
| export function loadConfig(cwd) { | ||||||||||||||
| cwd = cwd || process.cwd(); | ||||||||||||||
| const cached = _configCache.get(cwd); | ||||||||||||||
| if (cached) return structuredClone(cached); | ||||||||||||||
|
|
||||||||||||||
| for (const name of CONFIG_FILES) { | ||||||||||||||
| const filePath = path.join(cwd, name); | ||||||||||||||
| if (fs.existsSync(filePath)) { | ||||||||||||||
|
|
@@ -148,13 +155,26 @@ export function loadConfig(cwd) { | |||||||||||||
| merged.query.excludeTests = Boolean(config.excludeTests); | ||||||||||||||
| } | ||||||||||||||
| delete merged.excludeTests; | ||||||||||||||
| return resolveSecrets(applyEnvOverrides(merged)); | ||||||||||||||
| const result = resolveSecrets(applyEnvOverrides(merged)); | ||||||||||||||
| _configCache.set(cwd, result); | ||||||||||||||
| return result; | ||||||||||||||
| } catch (err) { | ||||||||||||||
| debug(`Failed to parse config ${filePath}: ${err.message}`); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| return resolveSecrets(applyEnvOverrides({ ...DEFAULTS })); | ||||||||||||||
| const defaults = resolveSecrets(applyEnvOverrides({ ...DEFAULTS })); | ||||||||||||||
| _configCache.set(cwd, defaults); | ||||||||||||||
| return defaults; | ||||||||||||||
|
Comment on lines
+166
to
+168
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The defaults code path has the same issue — the object stored in the cache is returned directly on the first call. This should also return a
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed. Same as above -- the merge with main brought in |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Clear the config cache. Intended for long-running processes that need to | ||||||||||||||
| * pick up on-disk config changes, and for test isolation when tests share | ||||||||||||||
| * the same cwd. | ||||||||||||||
| */ | ||||||||||||||
| export function clearConfigCache() { | ||||||||||||||
| _configCache.clear(); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const ENV_LLM_MAP = { | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the first call for a given
cwd,loadConfigreturns the actual cached object stored in_configCache. All subsequent calls correctly return astructuredClone. If any caller mutates the returned config (e.g., patchingllm.apiKeyor similar), the cached entry is silently corrupted and all future callers receive the mutated data.Both return sites after populating the cache should return a clone for consistency:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed. The merge with main already brought in the
structuredClonefix for both cache-population return sites (commit f8016c6 on main). The merged resolution preservesstructuredClone(result)when storing to cache, so the first caller gets the raw object and the cache holds an isolated copy.