Repository navigation
fix(di): scope the circular-dependency guard per-request, not application - #2344
Merged
Merged
Conversation
…tion
Symptom from fresh-VM tutorial run: the very first HTTP request to a
freshly-created Wheels app rendered a 1MB error page —
Wheels.DI.CircularDependency
Circular dependency detected while resolving 'app.controllers.Main'.
Resolution chain: app.controllers.Main -> app.controllers.Main
— with HTTP 200. Workaround was `wheels reload` then refresh; the
error never reappeared on subsequent boots.
Root cause: `variables.resolving` lived on the Injector instance,
which is stored at `application.wheelsdi`. That makes the resolving
struct application-scoped — shared across all concurrent request
threads. On a cold framework, when Lucee was still compiling
app/controllers/Main.cfc and two requests near-simultaneously hit
`getInstance("app.controllers.Main")`:
Thread A: getInstance("X") → adds to resolving → starts createObject
(slow on first cold compile)
Thread B: getInstance("X") → checks resolving → sees A's entry →
throws "Circular dependency: X -> X"
The `X -> X` chain is exactly what the journal saw — Thread B's
"chain" was just Thread A's in-flight entry. After `wheels reload`,
the CFC class is cached so the compile window closes and the race
narrows to nothing — matching the journal's observation that the
error never reappeared.
Fix: store the resolving stack on `request.$wheelsDIResolving`
instead of `variables.resolving`. Each request thread gets its own
struct, so concurrent calls don't trip each other's guard. Within a
single request, true circular dependencies still throw correctly —
the guard's intra-request semantics are unchanged.
`$getResolvingStack()` lazy-creates the request key on first access.
The `finally` block still pops the entry on both success and failure
so subsequent getInstance calls in the same request work as expected.
Two new InjectorSpec regressions:
- "resolving stack is request-scoped, not application-scoped (#2331)"
— verifies request.$wheelsDIResolving is created and that the
struct stays clean after a successful resolve.
- "resolving stack recovers cleanly after a circular-dep error (#2331)"
— verifies the finally cleanup runs even when a CircularDependency
is thrown, and that subsequent unrelated resolves work.
Fixes #2331.
Test plan:
- bash tools/test-local.sh — 3330 passed (was 3328; 2 new specs).
- bash tools/test-cli-local.sh — 452 pass / 3 fail (the 3 failures
are pre-existing Doctor Service issues #2260, unrelated).
- Existing "throws on circular dependency" spec still passes —
intra-request circular detection is unchanged.
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
The very first HTTP request to a freshly-created Wheels app rendered a 1MB error page —
```
Wheels.DI.CircularDependency
Circular dependency detected while resolving 'app.controllers.Main'.
Resolution chain: app.controllers.Main -> app.controllers.Main
```
— with HTTP 200. Workaround was `wheels reload` then refresh; the error never reappeared on subsequent boots.
Root cause
`variables.resolving` lived on the Injector instance, which is stored at `application.wheelsdi`. That makes the resolving struct application-scoped — shared across all concurrent request threads.
On a cold framework, when Lucee was still compiling `app/controllers/Main.cfc` and two requests near-simultaneously hit `getInstance("app.controllers.Main")`:
The `X -> X` chain in the error message is exactly what the journal saw — Thread B's resolution chain was just Thread A's in-flight entry being read across the thread boundary.
The `wheels reload` workaround works because reload clears Lucee's CFC class cache, but more importantly the SECOND attempt finds the class already compiled — the compile window closes and the race narrows to nothing. The journal's note that "the error never reappeared" matches.
Fix
Store the resolving stack on `request.$wheelsDIResolving` instead of `variables.resolving`. Each request thread gets its own struct, so concurrent calls don't trip each other's guard.
Tests
Two new `InjectorSpec` regressions:
The existing `throws on circular dependency` spec still passes — intra-request circular detection is unchanged.
Test plan
Closes #2331