Repository navigation
Defer custom-MRO subclass publication until initialization succeeds - #9527
youknowone merged 2 commits into
Conversation
Keep a type whose custom MRO failed out of its bases subclass lists until its MRO and slots are initialized. Register current bases under the type lock and reuse the cached callback-free weak reference to avoid duplicate registration after reentrant __bases__ assignment. Assisted-by: OpenAI Codex: model version unavailable
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Address clippy::use_self without changing initialization or locking behavior. Assisted-by: OpenAI Codex: model version unavailable
Merging this PR will not alter performance
Comparing 
Footnotes
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a02c6cbb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for base in typ.bases.read().as_slice() { | ||
| let mut subclasses = base.subclasses.write(); |
There was a problem hiding this comment.
If a custom mro() calls collections.abc.Sequence.register(Base) (or Mapping.register(Base)) while constructing Child(Base), deferring this link means _abc_register()'s set_abc_collection_flags_recursive() no longer reaches Child. new_heap_inner() copied the collection flags before the callback, and neither init_slots() nor update_subclass_flags_recursive() refreshes PyTypeFlags::COLLECTION, so the completed child keeps stale flags and sequence/mapping patterns silently fail to match its instances. The old early link propagated the registration correctly. Refresh the inherited collection flags after the callback and before publishing the type.
Useful? React with 👍 / 👎.
| let mut subclasses = base.subclasses.write(); | ||
| // A reentrant __bases__ assignment may have registered | ||
| // this type already. Callback-free weakrefs are cached. | ||
| if !subclasses.iter().any(|existing| existing.is(&weak)) { |
There was a problem hiding this comment.
When many dynamic classes are created with a custom metaclass—for example repeated typing.TypedDict(...) calls, which use dict as a physical base—this check scans the base's entire subclass history for each new class. GC leaves dead weakrefs in that vector until a pruning operation such as __subclasses__(), so collecting the classes does not prevent N creations from doing O(N²) comparisons where the previous publication path did O(N) appends. These scans also hold the interpreter-wide type lock. Preserve duplicate prevention for reentrant registration without scanning all prior subclasses on the normal construction path.
Useful? React with 👍 / 👎.
Summary
Fixes #9526.
Defer subclass-list publication for types using a custom metaclass until MRO and slot initialization succeed. A user can retain a type whose
mro()raised; publishing that still-incomplete type early lets the outer type's slot initialization reach its empty MRO.__set_name__/__init_subclass__callbacks, under the existing type lock.__bases__reentrancy and deduplicate the cached callback-free weak reference. This avoids a second subclass entry when a reentrant bases setter has already registered the type, without upgrading unrelated weak references while holding subclass locks.This follows CPython 3.15.0
type_ready_publish, which completes MRO/slot setup before publishing subclass links. CPython'sadd_subclassis idempotent by type identity. This does not claim that every externally retained type whose creation fails is unpublished: a reentrant__bases__operation may itself complete and publish an MRO before a later outer failure.Validation and limits
git diff --checkpassed.parking_lot_coregit checkout. This Draft relies on hosted CI for compilation and existing canonical tests.test_descr.MroTestincomplete-type and reentrant-bases tests are unchanged. In particular, canonicaltest_incomplete_extenddoes not keep the failed subclass alive, so its success alone does not establish the specific retained-child regression is fixed at runtime.