Skip to content

fix(di): key the Injector's circular-dependency guard per thread - #4449

Merged
bpamiri merged 1 commit into
developfrom
peter/4448-injector-cfthread-resolving
Oct 6, 2026
Merged

bpamiri merged 1 commit into
developfrom
peter/4448-injector-cfthread-resolving

Conversation

@bpamiri

@bpamiri bpamiri commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

The bug

The Injector's circular-dependency guard tracked in-flight names in a request-scoped struct (request.$wheelsDIResolving). On Lucee a cfthread shares its parent's request scope, so a request's threads shared one guard. While thread A was inside a slow init() of transient service X — holding X in the resolving stack — thread B's getInstance("X") saw X already present and threw a spurious Wheels.DI.CircularDependency (Resolution chain: X -> X). No cycle existed (#4448).

Fix

$getResolvingStack() now keys the stack by thread id within the request-scoped outer struct:

  • thread id via java.lang.Thread.currentThread().getId() (the same technique Migrator.$migrationLockThreadId() uses), falling back to "0" on a JVM-free engine;
  • the request-scoped outer struct is kept, preserving the per-request semantics that fixed the cross-request cold-start false positive (fix(di): Wheels.DI.CircularDependency on very first request to fresh app #2331);
  • lazy creation of the outer struct and each thread's sub-struct is guarded with lock scope="request" so a request's own threads don't race it (request-scoped, so no cross-request contention).

Test

The core runner can't see cfthread concurrency through an ordinary spec, so InjectorCfthreadResolvingSpec drives it deterministically (no timing sleeps): LatchedService.init() holds the first thread inside init() via java.util.concurrent latches until the second thread has passed the guard, then releases it.

  • RED on develop: Expected [ok] but received [Wheels.DI.CircularDependency].
  • GREEN with the fix.
  • Skips on a JVM-free engine without the latches; passes on engines whose cfthreads don't share the request scope (no bug there).

Verification

  • Full core suite (Lucee 7 + SQLite): 6993 pass, 0 fail, 0 error.
  • Injector area: 31/31 (RED-verified on base, green with fix).
  • Complexity gate: pass.

Shared plumbing (DI is used everywhere) + thread-scoping differs per engine, so the cross-engine matrix is warranted.

Closes #4448

🤖 Generated with Claude Code

The guard tracked in-flight names in a request-scoped struct shared by a request's
cfthreads (on Lucee a cfthread shares its parent's request scope). While one thread was
inside a slow init() of a transient service X — holding X in the resolving stack — another
thread's getInstance("X") saw X already present and threw a spurious
Wheels.DI.CircularDependency ("X -> X"). No cycle existed (#4448).

$getResolvingStack() now keys the stack by thread id (java.lang.Thread.getId(), as Migrator
already does; "0" on a JVM-free engine) within the request-scoped outer struct. That keeps
the per-request semantics that fixed the cross-request cold-start false positive (#2331)
while giving each thread its own set. The lazy creation of the outer struct and each
thread's sub-struct is guarded with lock scope="request" so a request's threads don't race it.

A new InjectorCfthreadResolvingSpec drives the concurrency deterministically:
LatchedService.init() holds the first thread inside init() (via java.util.concurrent
latches) until the second has passed the guard. RED on develop ("Expected [ok] but received
[Wheels.DI.CircularDependency]"), green with the fix; it skips on a JVM-free engine.

Closes #4448

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Review: head bb5e02a. Code APPROVE; the final waits on the compat matrix.

Cycle detection is unchanged; only the sharing changes.

The locks neither serialize nor deadlock. Both lock scope="request" blocks only guard the double-checked lazy creation: the outer struct once per request, and each thread's sub-struct once per thread. No lock is held across createObject / init() / onDIcomplete(). So:

  • unrelated resolutions run concurrently;
  • a nested resolve (A's init() needing B) re-enters $getResolvingStack(), takes the short lock and releases it, so it can't deadlock.

The thread-id source works on every JVM engine. CreateObject("java", "java.lang.Thread").currentThread().getId() is the same call Migrator.$migrationLockThreadId() already makes on Lucee, Adobe and BoxLang. java.lang is exported, so Adobe 2025's JPMS rule (invariant 14) doesn't apply. On a JVM-free engine the catch returns "0", which is the pre-fix behaviour, and fine there because cfthreads don't share a request scope.

The RED spec is deterministic (no sleeps):

  • t2 waits on started before it calls getInstance, so t1 is always the first constructor, held inside init() by proceed.
  • firstTaken.compareAndSet lets only that first construction block, so the fixed path (t2 constructs too) returns immediately.
  • t2's finally always releases t1, and both latches time out after 10 s, so a regression fails instead of hanging.
  • The threads are spawned from a component method, not an it() closure, which avoids the Lucee cfthread-in-closure compile error.
  • The spec's finally removes the application keys.

The RustCFML skip is justified. The spec needs java.util.concurrent latches, and on a JVM-free engine the fix itself falls back to the old shared key, so there's nothing to assert there.

INFO: $resolvingThreadId() does a CreateObject("java", ...) twice per construction (guard and cleanup), and transients construct on every getInstance. You could cache the java.lang.Thread class in variables once (lazily, with the same try/catch fallback) to keep that cost off the hot path. Not blocking.

PR CI and the full compat matrix (37391727495) are pending. The final follows once every leg is read and PR CI is green.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Wheels Test Results

228 724 tests   223 540 ✅  1h 37m 52s ⏱️
 18 112 suites    5 184 💤
     32 files          0 ❌

Results for commit bb5e02a.

@bpamiri

bpamiri commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

FINAL: APPROVE. Head bb5e02a.

  • Full compat matrix 37391727495 (on this head): 32/32 legs green, 0 fail, 0 error.
  • InjectorCfthreadResolvingSpec is 1/1 with 0 skipped on every leg: Lucee 6/7, Adobe 2023/2025 and BoxLang on all seven databases. The latch spec ran (no skip) and passed on every JVM engine with no timeouts. Lucee is the engine where cfthreads share the request scope, and so where the RED was demonstrated.
  • The injector, DI and full model/controller bundles are green alongside it, so per-thread keying doesn't disturb normal resolution.
  • PR CI: 18 pass, 0 fail, 0 pending, 3 skipping.
  • Open optional INFO: cache the java.lang.Thread class instead of a CreateObject("java", ...) twice per transient construction.

@bpamiri
bpamiri merged commit 0acd228 into develop Oct 6, 2026
59 checks passed
@bpamiri
bpamiri deleted the peter/4448-injector-cfthread-resolving branch October 6, 2026 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DI reports a false circular dependency when cfthreads resolve the same service concurrently

1 participant