Reuse reentrantly-registered type in mapAnnotatedObject (fixes first-request crash in long-lived workers)

[oojacoboo]

Problem

RecursiveTypeMapper throws RuntimeException: Cached type in registry is not the type returned by type mapper. on the first request after a fresh worker boot in long-lived runtimes (Swoole/RoadRunner/FrankenPHP); subsequent requests succeed. Reported as a new manifestation on #531 (by @apuckey): both instances are the same class (TypeAnnotatedObjectType) with the same GraphQL name but a different spl_object_id, and zero #[Input] types are involved.

Distinct from the 2022 bug fixed by #532 (which fixed #[Input] classes being read as #[Type] — different classes).

Root cause

TypeGenerator::mapAnnotatedObject() checks the registry before instantiating the annotated object via the container ($this->container->get(...)), but never after. Instantiating that object can reentrantly resolve and register this very type — e.g. a dependency in the container graph references it. The outer call, having already passed its pre-get() registry check, then builds a duplicate instance and returns it, which trips RecursiveTypeMapper's registry identity check.

It's first-request-only in long-lived workers because the in-memory TypeRegistry persists across requests and is warm after the first (failed) request. And it's output-only — InputTypeGenerator::mapFactoryMethod() already re-checks its cache after its own container->get(), and mapInput() has no container->get() — so mapAnnotatedObject() was the only generator missing the post-get() re-check.

Fix

Re-check the registry in mapAnnotatedObject() after container->get() and reuse the already-registered instance instead of building a duplicate — mirroring what mapFactoryMethod() already does for its own cache.

RecursiveTypeMapper's identity check is intentionally left untouched: with the duplicate no longer created, that check now serves as the invariant that verifies the fix — it would still throw if a duplicate ever slipped through again.

Tests

Adds a regression test that reproduces the reentrancy deterministically (a container whose get() registers the same type while the annotated object is being built) and asserts mapAnnotatedObject() reuses it. It fails on master (a duplicate is built) and passes with the fix. composer test is green locally (cs-check + PHPStan + PHPUnit).

Workaround for affected users on a release without this fix

Warm the schema once at worker bootstrap, before serving traffic — e.g. $schema->assertValid() — so the registry is fully populated and the race window never opens.

Refs #531

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.84%. Comparing base (53f9d49) to head (2e94838).
⚠️ Report is 159 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #812      +/-   ##
============================================
- Coverage     95.72%   94.84%   -0.88%     
- Complexity     1773     1894     +121     
============================================
  Files           154      176      +22     
  Lines          4586     4988     +402     
============================================
+ Hits           4390     4731     +341     
- Misses          196      257      +61     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[apuckey]

@oojacoboo amazing thank you this saved me some time :)

[oojacoboo]

@apuckey it'd be great if you could please test this PR before merging - let me know if this fully resolves it. You can add the commit directly to composer for testing. Here is a SO thread on the subject:

https://stackoverflow.com/questions/25878984/can-i-pull-a-specific-commit-with-composer

[apuckey]

will do first thing in the morning. many thanks

[apuckey]

@oojacoboo Good morning, This solved the initial problem but there is a similar one in the EnumTypeMapper::mapByClassName

It is creating a new type for each enum it finds instead of checking its been already created in the cachemap

this results in a different error but a similar error with multiple object ids if it happens you use the same enum in multiple types

note: i put this just after it sets the $typeName and everything works

if (isset($this->cacheByName[$typeName])) {
	return $this->cacheByName[$typeName];
}

the error comes from webonyx/graphql:

Schema must contain unique named types but contains multiple types named "LocationName" (see https://webonyx.github.io/graphql-php/type-definitions/#type-registry