refactor: deepen CacheItem to own the singleton get-or-create invariant - #314
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Internal deepening — no public-API or behaviour change. The singleton double-checked-lock (DCL) + memoization moves off
Factory.resolveonto a newCacheItem.get_or_createmethod, soFactory.resolvestops reaching intocache_item.cacheandcontainer._lockfield-by-field. This is Candidate 2 from the 2026-07-13 architecture review (Candidate 1 was the integration kit).Design/rationale:
planning/changes/2026-07-14.02-cacheitem-get-or-create.md.What changes
CacheItem.get_or_create(lock, resolve, create) -> (value, created)(registries/cache_registry.py) owns the full two-phase dance: fast-read ofself.cache, an unlockedresolve()(recursive kwargs resolution must not hold the lock), then acquire the lock, double-check,create(resolved), store, release. Returnscreatedso the caller knows whether to register for finalization.Factory.resolve(providers/factory.py) delegates to it and only drivesmark_createdvia thecreatedflag — it no longer acquires/releases the lock or writes the cache field.CreatorCallError/ArgumentResolutionErrorareResolutionErrorsubclasses, the resolve-timeprepend_stepdecoration moves into_resolve_kwargs(its only caller), so both thunks self-decorate andget_or_createcatches nothing — no double-prepended resolution step. Traces render byte-identically.Untouched: the close/finalize half of
CacheItem; the nogil-sensitivewiring_planpublication (stays unlocked);find_container/reopen; the non-caching path;mark_createdorchestration (stays a registry concern, no back-reference).Test plan
just test-ci— 100% line coverage, 379 tests. New direct unit tests forget_or_create(miss / hit / post-lock double-check / lock-release) intests/registries/test_cache_registry.py; zero changes to existing tests.just lint-ci— ruff (select=ALL),ty,check-planningclean.test_singleton_threading_concurrency(4-thread race) drives the inner double-check; reentrancy/deadlock anduse_lock=Falsetests pass unchanged (sameRLockpassed through).test_dependency_path+test_error_renderingpass unmodified.🤖 Generated with Claude Code