diff --git a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm index 22e2275f9c8e..b0bdfed944eb 100644 --- a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm +++ b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm @@ -37,11 +37,8 @@ @implementation RCTFabricSurface { // hence we wrap a value into `optional` to workaround it. std::optional _surfaceHandler; - // Protects Surface's start and stop processes. - // Even though SurfaceHandler is tread-safe, it will crash if we try to stop a surface that is not running. - // To make the API easy to use, we check the status of the surface before calling `start` or `stop`, - // and we need this mutex to prevent races. std::mutex _surfaceMutex; + BOOL _startPending; // Can be accessed from the main thread only. RCTSurfaceView *_Nullable _view; @@ -93,29 +90,36 @@ - (void)dealloc - (void)start { - std::lock_guard lock(_surfaceMutex); + { + std::lock_guard lock(_surfaceMutex); - if (_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered) { - return; + if (_startPending || _surfaceHandler->getStatus() != SurfaceHandler::Status::Registered) { + return; + } + + _startPending = YES; } - // We need to register a root view component here synchronously because right after - // we start a surface, it can initiate an update that can query the root component. RCTExecuteOnMainQueue(^{ - [self->_surfacePresenter.mountingManager attachSurfaceToView:self.view - surfaceId:self->_surfaceHandler->getSurfaceId()]; - dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{ - // This runs two async hops later; a concurrent instance teardown/reload may have unregistered - // the surface since the check in -start. Without this re-check SurfaceHandler::start() - // dereferences a now-null uiManager. - if (self->_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered) { + { + std::lock_guard lock(self->_surfaceMutex); + + if (!self->_startPending || self->_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered) { + self->_startPending = NO; return; } + + // We need to register a root view component here synchronously because right after + // we start a surface, it can initiate an update that can query the root component. + [self->_surfacePresenter.mountingManager attachSurfaceToView:self.view + surfaceId:self->_surfaceHandler->getSurfaceId()]; self->_surfaceHandler->start(); - [self _propagateStageChange]; + self->_startPending = NO; [self->_surfacePresenter setupAnimationDriverWithSurfaceHandler:*self->_surfaceHandler]; - }); + } + + [self _propagateStageChange]; }); } @@ -123,6 +127,8 @@ - (void)stop { std::lock_guard lock(_surfaceMutex); + _startPending = NO; + if (_surfaceHandler->getStatus() != SurfaceHandler::Status::Running) { return; } diff --git a/packages/react-native/React/Tests/Mounting/RCTFabricSurfaceLifecycleTests.mm b/packages/react-native/React/Tests/Mounting/RCTFabricSurfaceLifecycleTests.mm new file mode 100644 index 000000000000..006453c1b6fd --- /dev/null +++ b/packages/react-native/React/Tests/Mounting/RCTFabricSurfaceLifecycleTests.mm @@ -0,0 +1,82 @@ +/* + * 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 +#import +#import + +using facebook::react::ContextContainer; +using facebook::react::RuntimeExecutor; +using facebook::react::RuntimeScheduler; +using facebook::react::RuntimeSchedulerKey; +using facebook::react::SurfaceHandler; + +@interface RCTFabricSurfaceLifecycleTests : XCTestCase +@end + +@implementation RCTFabricSurfaceLifecycleTests { + std::shared_ptr _runtimeScheduler; + RCTSurfacePresenter *_surfacePresenter; +} + +- (void)setUp +{ + [super setUp]; + + RuntimeExecutor runtimeExecutor = [](std::function &&) {}; + _runtimeScheduler = std::make_shared(runtimeExecutor); + + auto contextContainer = std::make_shared(); + contextContainer->insert(RuntimeSchedulerKey, std::weak_ptr(_runtimeScheduler)); + + _surfacePresenter = [[RCTSurfacePresenter alloc] initWithContextContainer:contextContainer + runtimeExecutor:runtimeExecutor + bridgelessBindingsExecutor:std::nullopt]; +} + +- (void)tearDown +{ + [_surfacePresenter suspend]; + _surfacePresenter = nil; + _runtimeScheduler.reset(); + + [super tearDown]; +} + +- (void)testStopCancelsPendingStart +{ + RCTFabricSurface *surface = [[RCTFabricSurface alloc] initWithSurfacePresenter:_surfacePresenter + moduleName:@"" + initialProperties:@{}]; + XCTestExpectation *lifecycleSettled = [self expectationWithDescription:@"Deferred surface start settled"]; + dispatch_queue_t lifecycleQueue = dispatch_queue_create("RCTFabricSurfaceLifecycleTests", DISPATCH_QUEUE_SERIAL); + + XCTAssertTrue([NSThread isMainThread]); + dispatch_sync(lifecycleQueue, ^{ + [surface start]; + [surface stop]; + }); + dispatch_async(dispatch_get_main_queue(), ^{ + [lifecycleSettled fulfill]; + }); + + [self waitForExpectations:@[ lifecycleSettled ] timeout:2]; + + XCTAssertEqual(surface.surfaceHandler.getStatus(), SurfaceHandler::Status::Registered); + XCTAssertEqual(surface.view.subviews.count, 0u); + + [surface start]; + XCTAssertEqual(surface.surfaceHandler.getStatus(), SurfaceHandler::Status::Running); + + [surface stop]; + XCTAssertEqual(surface.surfaceHandler.getStatus(), SurfaceHandler::Status::Registered); +} + +@end