fix: prevent stack overflow in earlyResolve() on circular dependencies#2
Open
fix: prevent stack overflow in earlyResolve() on circular dependencies#2
Conversation
earlyResolve() runs during finishInit() — before finishLoad() where findGraphCycles() provides cycle detection. Without guards, circular dependencies among env flag items or @import enabled dependencies cause unbounded recursion and a process crash. Add an isResolved idempotency guard (matching resolve()), and a Set<string>-based cycle detection parameter following the same recursionStack DFS pattern used in findGraphCycles(). Wrap the DirectoryDataSource call site in try/catch so cycle errors surface as loading errors instead of uncaught exceptions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1cf2d26 to
56ba732
Compare
Captured from a real project (goalserve-poc demo-pwa) where APP_ENV=fallback($APP_ENV, $VARLOCK_ENV, "development") in .env.schema creates a self-referencing cycle that earlyResolve() follows infinitely. Log files: - varlock-oom-trace-1.log: OOM crash with 512MB Node heap cap - varlock-oom-trace-2.log: OOM crash with 256MB Node heap cap - varlock-env-info.log: Environment details (Node v22.22.1, varlock 0.4.1) - varlock-post-fix-trace.log: Clean error output after applying the fix Environment: Node v22.22.1, varlock 0.4.1, Linux x86_64, 8GB RAM Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
Real-world reproduction & verificationReproduced this bug in a real project (goalserve-poc demo-pwa) and confirmed the fix resolves it. Root causeThe project's
Before the fix
After the fixClean, immediate error: Environment
Full environment details in varlock-env-info.log. The fix was also verified by patching the compiled |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a stack overflow (
RangeError: Maximum call stack size exceeded) inConfigItem.earlyResolve()when the@currentEnvenv flag item or@import enableddependencies have circular references.earlyResolve()runs duringfinishInit()— beforefinishLoad()wherefindGraphCycles()provides cycle detection — so circular dependencies cause unbounded recursion and a process crashisResolvedidempotency guard (matching the existing guard inresolve()) to prevent redundant re-resolution on diamond dependenciesSet<string>-based cycle detection parameter, following the samerecursionStackDFS pattern already used infindGraphCycles()(graph-utils.ts)DirectoryDataSource._finishInit()call site in try/catch so cycle errors surface as_loadingErrorinstead of uncaught exceptionsChanges
packages/varlock/src/env-graph/lib/config-item.tsisResolvedguard + cycle detection via optionalresolvingSet param; fix typospackages/varlock/src/env-graph/lib/data-source.tsearlyResolve()at line 704 in try/catchpackages/varlock/src/env-graph/test/environments.test.tsTest plan
bunx vitest --run)APP_ENV=$APP_ENV) produces loading error, not crashAPP_ENV=$OTHER,OTHER=$APP_ENV) produces loading errorbun run lint:fix)🤖 Generated with Claude Code