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

changelog.d/4448-injector-cfthread-resolving.fixed.md

@@ -0,0 +1 @@
+- The dependency injector no longer throws a spurious `Wheels.DI.CircularDependency` when two of a request's `cfthread`s resolve the same transient (or request-scoped) service at the same time; the circular-dependency guard is now keyed per thread while keeping its per-request scope (#4448)

vendor/wheels/Injector.cfc

@@ -304,14 +304,12 @@ component implements="wheels.interfaces.di.InjectorInterface" {
 		required string componentPath,
 		required struct initArguments
 	) {
-		// Circular dependency guard. The resolving struct is request-scoped —
-		// it tracks "what this thread is currently resolving" — so concurrent
-		// requests don't trip each other's guard. Application-scoping it (the
-		// previous behavior) caused spurious self-loop errors when two
-		// requests near-simultaneously hit getInstance for the same controller
-		// on a cold framework, before Lucee had compiled the CFC. The chain
-		// "X -> X" in the error message was thread A's in-flight entry being
-		// observed by thread B before B even started. See issue #2331.
+		// Circular dependency guard. The resolving stack is per-thread (within a request-scoped outer
+		// struct) — so it tracks only what THIS thread is currently resolving. Application-scoping it
+		// (the original behavior) caused spurious "X -> X" self-loops when two requests near-
+		// simultaneously hit getInstance for the same controller on a cold framework (#2331); scoping
+		// it to the bare request scope then caused the same false positive between a request's own
+		// cfthreads, which share that scope on Lucee (#4448). Keying by thread id fixes both.
 		var resolving = $getResolvingStack();
 		if (structKeyExists(resolving, arguments.name)) {
 			throw(
@@ -368,17 +366,50 @@ component implements="wheels.interfaces.di.InjectorInterface" {
 	}
 
 	/**
-	 * Per-request resolving stack. Tracks which names are currently being
-	 * resolved in THIS thread/request, so the circular-dependency guard
-	 * doesn't see entries from concurrent requests.
+	 * Per-thread resolving stack. Tracks which names the CURRENT thread is resolving, so the
+	 * circular-dependency guard doesn't see entries from a concurrent request OR from a sibling
+	 * cfthread of the same request.
 	 *
-	 * Lazy-creates request.$wheelsDIResolving on first access.
+	 * The outer struct is request-scoped — that is what keeps a cold-start race between two requests
+	 * from reporting a false "X -> X" cycle (#2331). But on Lucee a `cfthread` shares its parent's
+	 * request scope, so a single request's threads shared one stack: while thread A was inside a slow
+	 * `init()` of service X, thread B's getInstance("X") saw X already present and threw a spurious
+	 * Wheels.DI.CircularDependency (#4448). Keying the stack by thread id gives each thread its own set
+	 * while keeping the request-scoped outer struct (and so #2331's semantics).
+	 *
+	 * The lazy creation of the shared outer struct and of each thread's sub-struct is locked so
+	 * concurrent threads of one request don't race it (one overwriting the other's in-flight entries).
 	 */
 	private struct function $getResolvingStack() {
-		if (!structKeyExists(request, "$wheelsDIResolving")) {
-			request.$wheelsDIResolving = {};
+		var tid = $resolvingThreadId();
+		if (!StructKeyExists(request, "$wheelsDIResolving")) {
+			lock scope="request" type="exclusive" timeout="10" {
+				if (!StructKeyExists(request, "$wheelsDIResolving")) {
+					request.$wheelsDIResolving = {};
+				}
+			}
+		}
+		if (!StructKeyExists(request.$wheelsDIResolving, tid)) {
+			lock scope="request" type="exclusive" timeout="10" {
+				if (!StructKeyExists(request.$wheelsDIResolving, tid)) {
+					request.$wheelsDIResolving[tid] = {};
+				}
+			}
+		}
+		return request.$wheelsDIResolving[tid];
+	}
+
+	/**
+	 * The current thread's id, or "0" on a JVM-free engine that has no java.lang.Thread (all such
+	 * threads then share one key — the pre-#4448 behaviour, acceptable where cfthreads don't share a
+	 * request scope anyway). Mirrors Migrator.$migrationLockThreadId().
+	 */
+	private string function $resolvingThreadId() {
+		try {
+			return CreateObject("java", "java.lang.Thread").currentThread().getId();
+		} catch (any e) {
+			return "0";
 		}
-		return request.$wheelsDIResolving;
 	}
 
 	/**

vendor/wheels/tests/_assets/di/LatchedService.cfc

@@ -0,0 +1,28 @@
+/**
+ * Test fixture for #4448: a transient service whose init() deterministically holds the Injector's
+ * circular-dependency guard open for one thread while a second thread checks it.
+ *
+ * The FIRST thread to construct it signals `started` (it is now inside init(), so its name is in the
+ * resolving stack) and then blocks on `proceed`; the second thread waits for `started`, does its own
+ * getInstance (which is where the spurious CircularDependency used to throw), then releases `proceed`.
+ * Latches live in application scope so they are shared whether or not a cfthread shares the parent's
+ * request scope. Only the first construction coordinates — later ones return immediately — so the
+ * post-fix path (both threads construct) cannot deadlock.
+ */
+component {
+
+	public any function init() {
+		if (StructKeyExists(application, "$di4448")) {
+			var state = {mine = application.$di4448.firstTaken.compareAndSet(JavaCast("boolean", false), JavaCast("boolean", true))};
+			if (state.mine) {
+				application.$di4448.started.countDown();
+				application.$di4448.proceed.await(
+					JavaCast("long", 10),
+					CreateObject("java", "java.util.concurrent.TimeUnit").SECONDS
+				);
+			}
+		}
+		return this;
+	}
+
+}

vendor/wheels/tests/specs/injector/InjectorCfthreadResolvingSpec.cfc

@@ -0,0 +1,90 @@
+/**
+ * #4448: the Injector's circular-dependency guard was keyed on 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 transient service X — holding X in the resolving stack — a second thread's
+ * getInstance("X") saw X already present and threw a spurious Wheels.DI.CircularDependency. Keying the
+ * stack by thread id fixes it while keeping the request-scoped outer struct's #2331 semantics.
+ *
+ * This is deterministic (no timing sleeps): LatchedService.init() holds the FIRST thread inside init()
+ * via a latch until the second thread has gone through the guard. Threads are spawned from a component
+ * method, not an it() closure — cfthread can't be declared in a closure on Lucee. The latches are
+ * java.util.concurrent, so the spec skips on an engine without them (e.g. a JVM-free one).
+ */
+component extends="wheels.WheelsTest" {
+
+	function run() {
+
+		describe("Injector circular-dependency guard across a request's cfthreads (4448)", () => {
+
+			it("does not throw a spurious CircularDependency when two threads resolve the same transient at once", () => {
+				if (!$di4448Supported()) {
+					skip("requires java.util.concurrent latches (JVM engine)");
+				}
+				var results = $runDiConcurrencyProbe();
+				expect(results.t1).toBe("ok");
+				expect(results.t2).toBe("ok");
+			});
+
+		});
+
+	}
+
+	private boolean function $di4448Supported() {
+		try {
+			CreateObject("java", "java.util.concurrent.CountDownLatch").init(JavaCast("int", 1));
+			return true;
+		} catch (any e) {
+			return false;
+		}
+	}
+
+	/**
+	 * Spawn two cfthreads that resolve the same transient service concurrently, overlapped so thread 2
+	 * passes the guard while thread 1 is inside the service's init(). Returns both thread results.
+	 */
+	private struct function $runDiConcurrencyProbe() {
+		application.$di4448 = {
+			started = CreateObject("java", "java.util.concurrent.CountDownLatch").init(JavaCast("int", 1)),
+			proceed = CreateObject("java", "java.util.concurrent.CountDownLatch").init(JavaCast("int", 1)),
+			firstTaken = CreateObject("java", "java.util.concurrent.atomic.AtomicBoolean").init(JavaCast("boolean", false))
+		};
+		var di = new wheels.Injector(binderPath = "wheels.tests._assets.di.TestBindings");
+		di.map("latched4448").to("wheels.tests._assets.di.LatchedService");
+		application.$di4448Injector = di;
+
+		try {
+			thread name="di4448_t1" action="run" {
+				thread.result = "unset";
+				try {
+					application.$di4448Injector.getInstance("latched4448");
+					thread.result = "ok";
+				} catch (any e) {
+					thread.result = e.type;
+				}
+			}
+			thread name="di4448_t2" action="run" {
+				thread.result = "unset";
+				try {
+					// Wait until t1 is inside init() (holding the guard), then resolve.
+					application.$di4448.started.await(
+						JavaCast("long", 10),
+						CreateObject("java", "java.util.concurrent.TimeUnit").SECONDS
+					);
+					application.$di4448Injector.getInstance("latched4448");
+					thread.result = "ok";
+				} catch (any e) {
+					thread.result = e.type;
+				} finally {
+					// Release t1 whether t2 passed or threw, so t1 always finishes.
+					application.$di4448.proceed.countDown();
+				}
+			}
+			thread action="join" name="di4448_t1,di4448_t2" timeout="20000";
+			return {t1 = cfthread.di4448_t1.result, t2 = cfthread.di4448_t2.result};
+		} finally {
+			StructDelete(application, "$di4448");
+			StructDelete(application, "$di4448Injector");
+		}
+	}
+
+}