feat: scope worker-created extended class names to their isolate - #428
Merged
Conversation
Named extends used the requested objc class name verbatim from every isolate, so a worker could win the registration race for a name the main isolate also uses (e.g. TimerTargetImpl via a shared module) and nondeterministically demote the main class to a collision suffix - breaking name-based native lookups that expect the main isolate's class. Append _<isolateId> to named extends from worker isolates. Main-isolate names stay byte-exact (the user-facing contract for storyboards and NSClassFromString), workers can no longer contend for them, and cross-isolate name collisions on fresh classes - the trigger of the +initialize deadlock fixed by the registration lock - become impossible by construction. Anonymous extends were already isolate-scoped. Behavior note: native code resolving a *worker's* class by its exact declared name now finds the scoped name instead; worker classes are internal to the worker model, so this is the isolation contract working as intended. Follow-up to #420.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe runtime now appends worker isolate IDs to explicitly named extended classes. Main-isolate and unnamed classes keep their existing names. The ChangesExtended class naming
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
GetExtendedClass took all three name strings by value and both call sites built the anonymous-name suffix on every extend even when unused. Pass const refs plus the isolate id and assemble the anonymous name inside only when needed (same output format); the worker scoping helper now appends in place (char + SSO to_string), adding zero heap allocations on the common path.
NathanWalker
approved these changes
Aug 4, 2026
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
2 participants
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.
Follow-up to #420 (proposed there as the by-construction hardening after the registration lock in #421).
Problem
Named extends register the requested objc class name verbatim from every isolate, and the objc class namespace is process-global. A worker that extends a shared named class (e.g.
TimerTargetImplviaInfrastructure/timers) before the main isolate does wins the registration race: the worker owns the bare name and the main isolate's class is nondeterministically demoted to a collision suffix (TimerTargetImpl1). Any name-based native lookup expecting the main isolate's class (NSClassFromString, storyboard class references, crash symbolication) then resolves the worker's. Cross-isolate contention for fresh same-named classes was also the trigger of the+initializedeadlock (#420) — serialized by #421's lock, now impossible to reach at all.Change
Named extends from worker isolates get
_<isolateId>appended (ScopeClassNameToIsolate, applied on both extend paths — explicit-name.extend()and native__extends). Main-isolate names stay byte-exact — the user-facing contract — and are now deterministic regardless of worker startup order, since workers can no longer contend for them. Anonymous extends were already isolate-scoped.Behavior note: native code resolving a worker's class by its exact declared name now finds the scoped name instead. Worker-created classes are internal to the worker model, so this is the isolation contract working as intended (and
NSStringFromClassround-trips are unaffected — they use the actual registered name).Tests
New
ExtendedClassNamingTests(registered in the runner):__extendspath scopes too.Full local suite green (0 failures), including the three new specs.
Summary by CodeRabbit