diff --git a/packages/react-native/React/Fabric/RCTScheduler.h b/packages/react-native/React/Fabric/RCTScheduler.h index 214e43b3010..8725c1956c7 100644 --- a/packages/react-native/React/Fabric/RCTScheduler.h +++ b/packages/react-native/React/Fabric/RCTScheduler.h @@ -62,6 +62,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)setupAnimationDriver:(const facebook::react::SurfaceHandler &)surfaceHandler; +// Permanently removes the layout-animation observer. Call on the main queue. +- (void)stopObservingLayoutAnimations; + - (void)onAnimationStarted; - (void)onAllAnimationsComplete; diff --git a/packages/react-native/React/Fabric/RCTScheduler.mm b/packages/react-native/React/Fabric/RCTScheduler.mm index 9519f18428e..5130b2fdcdd 100644 --- a/packages/react-native/React/Fabric/RCTScheduler.mm +++ b/packages/react-native/React/Fabric/RCTScheduler.mm @@ -8,6 +8,7 @@ #import "RCTScheduler.h" #import +#import #import #import #import @@ -258,6 +259,12 @@ - (void)setupAnimationDriver:(const facebook::react::SurfaceHandler &)surfaceHan surfaceHandler.getMountingCoordinator()->setMountingOverrideDelegate(_animationDriver); } +- (void)stopObservingLayoutAnimations +{ + RCTAssertMainQueue(); + _uiRunLoopObserver.reset(); +} + - (void)onAnimationStarted { if (_uiRunLoopObserver) { diff --git a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm index e5b19bd4765..e3f6424f53f 100644 --- a/packages/react-native/React/Fabric/RCTSurfacePresenter.mm +++ b/packages/react-native/React/Fabric/RCTSurfacePresenter.mm @@ -234,6 +234,7 @@ - (BOOL)suspend } [self _stopAllSurfacesWithScheduler:scheduler]; + scheduler.delegate = nil; return YES; } diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/React-RuntimeApple.podspec b/packages/react-native/ReactCommon/react/runtime/platform/ios/React-RuntimeApple.podspec index 6b4a055574b..445e6af7f05 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/React-RuntimeApple.podspec +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/React-RuntimeApple.podspec @@ -18,6 +18,7 @@ end header_search_paths = [ "$(PODS_ROOT)/Headers/Private/React-Core", + "$(PODS_ROOT)/Headers/Private/Yoga", "$(PODS_TARGET_SRCROOT)/../../../..", "$(PODS_TARGET_SRCROOT)/../../../../..", ] @@ -50,6 +51,7 @@ Pod::Spec.new do |s| s.dependency "React-CoreModules" s.dependency "React-NativeModulesApple" s.dependency "React-RCTFabric" + s.dependency "Yoga" s.dependency "React-RuntimeCore" s.dependency "React-Mapbuffer" s.dependency "React-jserrorhandler" diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm index 2890e6cb5d5..45d2712b968 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm @@ -29,6 +29,7 @@ #import #import #import +#import #import #import #import @@ -191,28 +192,36 @@ - (void)invalidate } // suspend shouldn't be called under _invalidationMutex: it notifies surface observers, // which can call callFunctionOnJSModule and deadlock on the same mutex. + // Queued rendering updates capture the scheduler's delegate by raw pointer. + // Stop its run-loop observer on main, but keep it alive until queued JS work + // has drained. Release it before the runtime so its queued JSI handles remain valid. + __block RCTScheduler *scheduler = _surfacePresenter.scheduler; [_surfacePresenter suspend]; - [_jsThreadManager dispatchToJSThread:^{ - /** - * Every TurboModule is invalidated on its own method queue. - * TurboModuleManager invalidate blocks the calling thread until all TurboModules are invalidated. - */ - [self->_turboModuleManager invalidate]; - - // Clean up all the Resources - self->_reactInstance = nullptr; - self->_jsRuntimeFactory = nullptr; - self->_appTMMDelegate = nil; - self->_delegate = nil; - [self->_displayLink invalidate]; - self->_displayLink = nil; - - self->_turboModuleManager = nil; - self->_performanceLogger = nil; - - // Terminate the JavaScript thread, so that no other work executes after this block. - self->_jsThreadManager = nil; - }]; + RCTExecuteOnMainQueue(^{ + [scheduler stopObservingLayoutAnimations]; + [self->_jsThreadManager dispatchToJSThread:^{ + /** + * Every TurboModule is invalidated on its own method queue. + * TurboModuleManager invalidate blocks the calling thread until all TurboModules are invalidated. + */ + [self->_turboModuleManager invalidate]; + + // Clean up all the Resources + scheduler = nil; + self->_reactInstance = nullptr; + self->_jsRuntimeFactory = nullptr; + self->_appTMMDelegate = nil; + self->_delegate = nil; + [self->_displayLink invalidate]; + self->_displayLink = nil; + + self->_turboModuleManager = nil; + self->_performanceLogger = nil; + + // Terminate the JavaScript thread, so that no other work executes after this block. + self->_jsThreadManager = nil; + }]; + }); } - (std::shared_ptr)jsCallInvoker diff --git a/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj b/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj index adc9a277d08..b9400e014b9 100644 --- a/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj +++ b/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj @@ -50,6 +50,7 @@ F0D621C32BBB9E38005960AC /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = F0D621C22BBB9E38005960AC /* PrivacyInfo.xcprivacy */; }; F1A0B1C23D4E5F6071829302 /* RCTTurboModuleArrayBufferTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F1A0B1C23D4E5F6071829301 /* RCTTurboModuleArrayBufferTests.mm */; }; F1A0B1C23D4E5F6071829322 /* RCTTurboModuleStringConversionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F1A0B1C23D4E5F6071829321 /* RCTTurboModuleStringConversionTests.mm */; }; + F1A0B1C23D4E5F6071829342 /* RCTInstanceInvalidationTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F1A0B1C23D4E5F6071829341 /* RCTInstanceInvalidationTests.mm */; }; D1A60AD11B9247A080734F02 /* RCTDevLoadingViewTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D1A60AD11B9247A080734F01 /* RCTDevLoadingViewTests.m */; }; /* End PBXBuildFile section */ @@ -148,6 +149,7 @@ F0D621C22BBB9E38005960AC /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = ""; }; F1A0B1C23D4E5F6071829301 /* RCTTurboModuleArrayBufferTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTTurboModuleArrayBufferTests.mm; sourceTree = ""; }; F1A0B1C23D4E5F6071829321 /* RCTTurboModuleStringConversionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTTurboModuleStringConversionTests.mm; sourceTree = ""; }; + F1A0B1C23D4E5F6071829341 /* RCTInstanceInvalidationTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RCTInstanceInvalidationTests.mm; sourceTree = ""; }; D1A60AD11B9247A080734F01 /* RCTDevLoadingViewTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RCTDevLoadingViewTests.m; sourceTree = ""; }; /* End PBXFileReference section */ @@ -309,6 +311,7 @@ E7DB20CB22B2BAA5005AC45F /* RCTModuleMethodTests.mm */, F1A0B1C23D4E5F6071829301 /* RCTTurboModuleArrayBufferTests.mm */, F1A0B1C23D4E5F6071829321 /* RCTTurboModuleStringConversionTests.mm */, + F1A0B1C23D4E5F6071829341 /* RCTInstanceInvalidationTests.mm */, E7DB20CF22B2BAA5005AC45F /* RCTMultipartStreamReaderTests.m */, A975CA6B2C05EADE0043F72A /* RCTNetworkTaskTests.m */, E7DB20BE22B2BAA4005AC45F /* RCTNativeAnimatedNodesManagerTests.m */, @@ -620,6 +623,7 @@ E7DB20E822B2BAA6005AC45F /* RCTModuleMethodTests.mm in Sources */, F1A0B1C23D4E5F6071829302 /* RCTTurboModuleArrayBufferTests.mm in Sources */, F1A0B1C23D4E5F6071829322 /* RCTTurboModuleStringConversionTests.mm in Sources */, + F1A0B1C23D4E5F6071829342 /* RCTInstanceInvalidationTests.mm in Sources */, E7DB20E222B2BAA6005AC45F /* RCTGzipTests.m in Sources */, E7DB20ED22B2BAA6005AC45F /* RCTURLUtilsTests.m in Sources */, E7DB20D322B2BAA6005AC45F /* RCTBlobManagerTests.m in Sources */, diff --git a/packages/rn-tester/RNTesterUnitTests/RCTInstanceInvalidationTests.mm b/packages/rn-tester/RNTesterUnitTests/RCTInstanceInvalidationTests.mm new file mode 100644 index 00000000000..4ddd151f15c --- /dev/null +++ b/packages/rn-tester/RNTesterUnitTests/RCTInstanceInvalidationTests.mm @@ -0,0 +1,196 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import +#import + +#import +#import + +@interface RCTSchedulerLifetimeProbe : NSObject +@property (nonatomic, weak) id delegate; +@property (nonatomic, copy) void (^onDealloc)(BOOL onMainThread); +@property (nonatomic, copy) void (^onStopObserving)(BOOL onMainThread); +@property (nonatomic, copy) dispatch_block_t onTick; +- (void)startObserving; +- (void)stopObservingLayoutAnimations; +@end + +@implementation RCTSchedulerLifetimeProbe { + CFRunLoopObserverRef _observer; +} + +- (void)startObserving +{ + void *unretainedSelf = (__bridge void *)self; + _observer = CFRunLoopObserverCreateWithHandler( + kCFAllocatorDefault, kCFRunLoopBeforeWaiting, true, 0, ^(CFRunLoopObserverRef, CFRunLoopActivity) { + __unsafe_unretained RCTSchedulerLifetimeProbe *probe = (__bridge RCTSchedulerLifetimeProbe *)unretainedSelf; + dispatch_block_t tick = probe.onTick; + if (tick) { + tick(); + } + }); + CFRunLoopAddObserver(CFRunLoopGetMain(), _observer, kCFRunLoopCommonModes); +} + +- (void)stopObservingLayoutAnimations +{ + if (_observer) { + CFRunLoopRemoveObserver(CFRunLoopGetMain(), _observer, kCFRunLoopCommonModes); + CFRelease(_observer); + _observer = nullptr; + } + if (_onStopObserving) { + _onStopObserving([NSThread isMainThread]); + } +} + +- (void)dealloc +{ + if (_observer) { + CFRunLoopRemoveObserver(CFRunLoopGetMain(), _observer, kCFRunLoopCommonModes); + CFRelease(_observer); + } + if (_onDealloc) { + _onDealloc([NSThread isMainThread]); + } +} +@end + +@interface RCTJSThreadManagerStub : NSObject +@property (nonatomic, copy) dispatch_block_t pendingWork; +@end + +@implementation RCTJSThreadManagerStub +- (void)dispatchToJSThread:(dispatch_block_t)work +{ + self.pendingWork = work; +} +@end + +@interface RCTInstanceInvalidationTests : XCTestCase +@end + +@implementation RCTInstanceInvalidationTests + +- (void)runJSWork:(dispatch_block_t)work +{ + dispatch_semaphore_t finished = dispatch_semaphore_create(0); + dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0), ^{ + @autoreleasepool { + work(); + } + dispatch_semaphore_signal(finished); + }); + XCTAssertEqual(dispatch_semaphore_wait(finished, dispatch_time(DISPATCH_TIME_NOW, 3 * NSEC_PER_SEC)), 0); +} + +- (void)testInvalidateStopsObserverOnMainAndReleasesSchedulerDuringJSCleanup +{ + XCTAssertTrue([NSThread isMainThread]); + RCTSurfacePresenter *presenter = [RCTSurfacePresenter new]; + RCTJSThreadManagerStub *jsThreadManager = [RCTJSThreadManagerStub new]; + RCTInstance *instance = [RCTInstance new]; + [instance setValue:presenter forKey:@"surfacePresenter"]; + [instance setValue:jsThreadManager forKey:@"jsThreadManager"]; + + XCTestExpectation *released = [self expectationWithDescription:@"Scheduler released during JS cleanup"]; + __block BOOL observerStoppedOnMain = NO; + __block BOOL releasedBeforeJSThreadCleanup = NO; + __weak RCTInstance *weakInstance = instance; + __weak RCTSchedulerLifetimeProbe *weakScheduler; + @autoreleasepool { + RCTSchedulerLifetimeProbe *scheduler = [RCTSchedulerLifetimeProbe new]; + scheduler.delegate = presenter; + scheduler.onStopObserving = ^(BOOL onMainThread) { + observerStoppedOnMain = onMainThread; + }; + scheduler.onDealloc = ^(BOOL) { + releasedBeforeJSThreadCleanup = [weakInstance valueForKey:@"jsThreadManager"] != nil; + [released fulfill]; + }; + weakScheduler = scheduler; + // Exercise real instance invalidation and presenter suspension without a JS VM. + [presenter setValue:scheduler forKey:@"scheduler"]; + } + + @autoreleasepool { + [instance invalidate]; + XCTAssertNil(presenter.scheduler); + XCTAssertNotNil(weakScheduler); + XCTAssertNil(weakScheduler.delegate); + XCTAssertNotNil(jsThreadManager.pendingWork); + XCTAssertTrue(observerStoppedOnMain); + } + + dispatch_block_t drainWork = jsThreadManager.pendingWork; + [self runJSWork:drainWork]; + + [self waitForExpectations:@[ released ] timeout:3]; + XCTAssertTrue(releasedBeforeJSThreadCleanup); + // Keeping the cleanup block itself must not keep the scheduler alive. + XCTAssertNil(weakScheduler); + XCTAssertNil([instance valueForKey:@"jsThreadManager"]); + jsThreadManager.pendingWork = nil; +} + +- (void)testStoppedObserverDoesNotRunWhileWaitingForJSCleanup +{ + XCTAssertTrue([NSThread isMainThread]); + RCTSurfacePresenter *presenter = [RCTSurfacePresenter new]; + RCTJSThreadManagerStub *jsThreadManager = [RCTJSThreadManagerStub new]; + RCTInstance *instance = [RCTInstance new]; + [instance setValue:presenter forKey:@"surfacePresenter"]; + [instance setValue:jsThreadManager forKey:@"jsThreadManager"]; + + XCTestExpectation *noTick = [self expectationWithDescription:@"Stopped observer must not run"]; + noTick.inverted = YES; + XCTestExpectation *released = [self expectationWithDescription:@"Scheduler released during JS cleanup"]; + __weak RCTSchedulerLifetimeProbe *weakScheduler; + @autoreleasepool { + RCTSchedulerLifetimeProbe *scheduler = [RCTSchedulerLifetimeProbe new]; + weakScheduler = scheduler; + scheduler.onDealloc = ^(BOOL) { + [released fulfill]; + }; + scheduler.onTick = ^{ + [noTick fulfill]; + }; + [presenter setValue:scheduler forKey:@"scheduler"]; + [scheduler startObserving]; + } + + @autoreleasepool { + [instance invalidate]; + } + [self waitForExpectations:@[ noTick ] timeout:0.05]; + XCTAssertNotNil(weakScheduler); + [self runJSWork:jsThreadManager.pendingWork]; + [self waitForExpectations:@[ released ] timeout:3]; + XCTAssertNil(weakScheduler); + XCTAssertNil([instance valueForKey:@"jsThreadManager"]); + jsThreadManager.pendingWork = nil; +} + +- (void)testInvalidateWithoutSchedulerCompletesJSThreadCleanup +{ + RCTSurfacePresenter *presenter = [RCTSurfacePresenter new]; + RCTJSThreadManagerStub *jsThreadManager = [RCTJSThreadManagerStub new]; + RCTInstance *instance = [RCTInstance new]; + [instance setValue:presenter forKey:@"surfacePresenter"]; + [instance setValue:jsThreadManager forKey:@"jsThreadManager"]; + + [instance invalidate]; + XCTAssertNotNil(jsThreadManager.pendingWork); + jsThreadManager.pendingWork(); + XCTAssertNil(presenter.scheduler); + XCTAssertNil([instance valueForKey:@"jsThreadManager"]); + jsThreadManager.pendingWork = nil; +} + +@end diff --git a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api index 95b044c2bcb..3be0ca69619 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api @@ -1592,6 +1592,7 @@ interface RCTScheduler : public NSObject { public virtual void removeEventListener:(const std::shared_ptr& listener); public virtual void reportMount:(facebook::react::SurfaceId surfaceId); public virtual void setupAnimationDriver:(const facebook::react::SurfaceHandler& surfaceHandler); + public virtual void stopObservingLayoutAnimations(); public virtual void unregisterSurface:(const facebook::react::SurfaceHandler& surfaceHandler); } diff --git a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api index e2574b7a550..30c3e306625 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api @@ -1635,6 +1635,7 @@ interface RCTScheduler : public NSObject { public virtual void removeEventListener:(const std::shared_ptr& listener); public virtual void reportMount:(facebook::react::SurfaceId surfaceId); public virtual void setupAnimationDriver:(const facebook::react::SurfaceHandler& surfaceHandler); + public virtual void stopObservingLayoutAnimations(); public virtual void unregisterSurface:(const facebook::react::SurfaceHandler& surfaceHandler); } diff --git a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api index d05f330f190..269af275501 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api @@ -1591,6 +1591,7 @@ interface RCTScheduler : public NSObject { public virtual void removeEventListener:(const std::shared_ptr& listener); public virtual void reportMount:(facebook::react::SurfaceId surfaceId); public virtual void setupAnimationDriver:(const facebook::react::SurfaceHandler& surfaceHandler); + public virtual void stopObservingLayoutAnimations(); public virtual void unregisterSurface:(const facebook::react::SurfaceHandler& surfaceHandler); } diff --git a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api index 01006d9de4d..4f7249765a8 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api @@ -1592,6 +1592,7 @@ interface RCTScheduler : public NSObject { public virtual void removeEventListener:(const std::shared_ptr& listener); public virtual void reportMount:(facebook::react::SurfaceId surfaceId); public virtual void setupAnimationDriver:(const facebook::react::SurfaceHandler& surfaceHandler); + public virtual void stopObservingLayoutAnimations(); public virtual void unregisterSurface:(const facebook::react::SurfaceHandler& surfaceHandler); }