Conversation
safaiyeh
marked this pull request as ready for review
September 28, 2026 06:18
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:
Fabric's Scheduler queues rendering callbacks that capture a borrowed
SchedulerDelegate*. Detaching the delegate or destroying the Scheduler does not revoke those captures. On iOS, RCTScheduler owns the delegate proxy, so pending commands or mounting work can call through a released proxy after ordinary host teardown. Error-time queue clearing does not cover this non-error path; the remaining gap is also described in 9682967a09 / #58138.This change gives each delegate assignment its own registration. Queued callbacks acquire a lease when they execute. Retirement rejects new leases; an already acquired owned lease keeps its target alive until it returns. Replacement creates a separate registration so old queued work cannot target the replacement. Delegate calls and final releases occur outside synchronization primitives.
The iOS adapter passes a shared proxy into the Scheduler and uses a zeroing weak RCTScheduler backpointer, promoted locally before forwarding. This avoids both a dangling Objective-C owner and a retain cycle. The change covers deferred commands, transactions and React-revision merging, plus direct delegate calls.
The raw-pointer constructor/setter remain available for Android and existing integrations. They cancel retired queued work but retain their caller-managed lifetime contract for callbacks already admitted. Each assignment starts a new generation, even when the pointer is unchanged. Lifecycle mutations remain externally serialized; callbacks may overlap retirement. This does not claim to solve every RuntimeScheduler shutdown or surface-unregistration lifetime issue.
Changelog:
[IOS] [FIXED] - Prevent queued Fabric rendering callbacks from using a retired Scheduler delegate, and retain owned delegates while active callbacks finish.
Test Plan:
RCTScheduler.mmpasses Objective-C++ ARC syntax compilation against upstream headers and the iOS Simulator SDK.Remaining validation: full RNTester/Expo host reload and termination tests, UIKit teardown-thread behavior under overlapping callbacks, and assessment of the additional shared-ownership operations on delegate hot paths. The ARC overlap test observes that the final owner release can occur on the callback thread. The isolated native tests establish the delegate ownership/cancellation contract, not complete application shutdown safety. Android still uses the borrowed delegate contract.
Local native validation commands and scope
The standalone CMake harness compiles the actual modified
Scheduler.cpp, its ReactCommon dependency closure, and the checked-in regression tests. It uses Apple Clang 21, C++20, GoogleTest 1.17.0 (52eb8108c5bdec04579160ae17225d66034bd723), Hermes macOS package 250829098.0.17, Folly 0.58.0-dev, fmt 12.2.0 and glog 0.7.1. It uses the real Root/View component registry; the animation backend's unusedreact_codegen_rncoredependency is an empty interface target in this isolated harness. This is local source validation, not an official RNTester or Android build.Sanitizer builds instrument the production registration header, helper tests and GoogleTest; prebuilt dependency internals remain unsanitized. The Scheduler-destruction test deliberately retains UIManager while draining callbacks, because the runtime's separate revision-manager pointer lifetime is outside this patch.