Skip to content

Commit f1488ec

Browse files
dmiliutsfacebook-github-bot
authored andcommitted
Guard deferred Fabric surface start against concurrent unregister (#57404)
Summary: `-[RCTFabricSurface start]` defers `SurfaceHandler::start()` across two async hops after its synchronous `Registered` check. If a concurrent `RCTInstance` teardown unregisters the surface in that window, the deferred `start()` dereferences a now-null `link_.uiManager` — debug aborts on the `react_native_assert`, release null-derefs. It is a TOCTOU: `-[RCTInstance invalidate]` tears down asynchronously on the JS thread with no completion signal, so an app-layer barrier cannot order against it — the start path itself must tolerate a concurrently-unregistered surface. Fix: re-check `getStatus() == Registered` inside the deferred block before calling `start()`. Behavior-preserving on the success path; only the already-broken unregistered case changes from crash to a cancelled start. Scope: iOS only. A companion cross-platform guard in `SurfaceHandler::start()` (return + `LOG(ERROR)` instead of the dev-fatal/release-compiled-out assert) would close the residual window between the re-check and `start()` atomically; left out to keep this iOS-only — happy to add if reviewers prefer. Flagging for code-owner review since this is core surface-start code. Repro: an in-process React host teardown + rebuild that unregisters a surface while its deferred start is still queued. Changelog: [iOS][Fixed] - Cancel a deferred Fabric surface start when the surface was unregistered by a concurrent instance teardown, instead of dereferencing a null UIManager Differential Revision: D109477868
1 parent 923e7dd commit f1488ec

1 file changed

Lines changed: 6 additions & 0 deletions

File tree

packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,12 @@ - (void)start
105105
[self->_surfacePresenter.mountingManager attachSurfaceToView:self.view
106106
surfaceId:self->_surfaceHandler->getSurfaceId()];
107107
dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{
108+
// This runs two async hops later; a concurrent instance teardown/reload may have unregistered
109+
// the surface since the check in -start. Without this re-check SurfaceHandler::start()
110+
// dereferences a now-null uiManager.
111+
if (self->_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered) {
112+
return;
113+
}
108114
self->_surfaceHandler->start();
109115
[self _propagateStageChange];
110116

0 commit comments

Comments
 (0)