From 54e9622067c82c7e5dd9d2c4f4226e527e958636 Mon Sep 17 00:00:00 2001 From: Daewoon Kim Date: Mon, 28 Sep 2026 12:13:15 +0900 Subject: [PATCH 1/3] =?UTF-8?q?fix(scatterlab):=20=EA=B2=B9=EC=B9=9C=20sur?= =?UTF-8?q?face=20start=20=EA=B0=80=20ShadowTree=20=ED=8F=AC=EC=9D=B8?= =?UTF-8?q?=ED=84=B0=EB=A5=BC=20=EB=8C=95=EA=B8=80=EB=A7=81=EC=8B=9C?= =?UTF-8?q?=ED=82=A4=EC=A7=80=20=EC=95=8A=EA=B2=8C=20=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `-[RCTFabricSurface start]` 는 status 가 Registered 인지만 보고, main → global 두 번의 async hop 뒤에야 `SurfaceHandler::start()` 를 부른다. 그 사이 status 는 계속 Registered 라서 `RCTHost` 의 버퍼된 시작(createSurface, reload 재시작)·`RCTSurfacePresenter` resume 이 겹치면 start 가 두 번 돈다. 두 번째 호출은 같은 SurfaceId 로 새 ShadowTree 를 만들어 `ShadowTreeRegistry::add()` 에 넘기는데, emplace 가 중복 키를 무시하므로 그 트리는 곧바로 파괴되고 `link_.shadowTree` 만 그걸 가리킨 채 남는다. 이어지는 `setupAnimationDriver` 가 `getMountingCoordinator()` 로 해제된 트리를 읽고 `setMountingOverrideDelegate` 의 mutex(+0x8)에서 SIGSEGV 0x8 로 죽는다. release 에선 `react_native_assert` 가 컴파일 아웃돼 이걸 막는 게 없다. - `SurfaceHandler::start()` — link mutex 안에서 status 가 Registered 가 아니면 `LOG(WARNING)` 후 반환한다. status assert 는 이 분기로 대체했다. 경합은 호출자 오용이 아니라 RN 자신의 타이밍이라 dev 빌드를 죽일 이유가 없다. Android 도 같은 C++ 를 탄다. - `RCTFabricSurface` — `_startInFlight` 로 앞선 start 가 끝나기 전의 두 번째 `-start` 를 막는다(`attachSurfaceToView` 중복, override delegate 중복 등록 방지). deferred 블록은 start 로 Running 이 됐을 때만 animation driver 를 붙인다. 그 사이 unregister 되면 start 는 no-op 이고 MountingCoordinator 를 꺼낼 트리가 없다. 상류 #57404(open)는 unregister 경합만 iOS 쪽 재확인으로 막는다. 이 수정은 그 경우를 포함한다. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WXbb9Sp9jdMsrnBGjzVdgP --- .github/scatterlab/allowed-tarball-diff.txt | 12 ++++++++++++ .../React/Fabric/Surface/RCTFabricSurface.mm | 15 +++++++++++++-- .../react/renderer/scheduler/SurfaceHandler.cpp | 14 ++++++++++++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/.github/scatterlab/allowed-tarball-diff.txt b/.github/scatterlab/allowed-tarball-diff.txt index 77ce3ae7f323..bfd0420bf680 100644 --- a/.github/scatterlab/allowed-tarball-diff.txt +++ b/.github/scatterlab/allowed-tarball-diff.txt @@ -55,3 +55,15 @@ ReactAndroid/src/main/java/com/facebook/react/views/text/TextDecorationStyle.kt # Points the consumer's Gradle build at this fork's Android artifacts. Added file, so the # gate sees it as a difference from upstream. See android-prebuilt.md. scripts/android/scatterlab-prebuilt-maven.gradle + +# A second SurfaceHandler::start() on a running surface hands ShadowTreeRegistry::add() +# another tree for the same SurfaceId; the registry drops it and `link_.shadowTree` is +# left dangling, so the next MountingCoordinator lookup crashes (iOS: SIGSEGV at 0x8 in +# MountingCoordinator::setMountingOverrideDelegate). -[RCTFabricSurface start] checks +# the status two async hops before start() runs, so racing callers get there twice. +# start() now ignores a surface that is not Registered, and the iOS surface keeps a +# second start out while one is in flight. Upstream +# https://github.com/react/react-native/pull/57404 (open) covers the unregistered case +# on the iOS side only. C++ reaches Android only through this fork's Android prebuilt. +ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp +React/Fabric/Surface/RCTFabricSurface.mm diff --git a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm index 1a96fef72ae2..8b8a8da85353 100644 --- a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm +++ b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm @@ -7,6 +7,7 @@ #import "RCTFabricSurface.h" +#import #import #import @@ -43,6 +44,11 @@ @implementation RCTFabricSurface { // and we need this mutex to prevent races. std::mutex _surfaceMutex; + // `start` returns before `SurfaceHandler::start()` runs (it hops to the main queue, then to a global one), and + // until then the status still reads `Registered`. This keeps a second `start` in that window from attaching the + // root view and starting the surface again. + std::atomic_bool _startInFlight; + // Can be accessed from the main thread only. RCTSurfaceView *_Nullable _view; #if !TARGET_OS_TV @@ -95,7 +101,7 @@ - (void)start { std::lock_guard lock(_surfaceMutex); - if (_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered) { + if (_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered || _startInFlight.exchange(true)) { return; } @@ -108,7 +114,12 @@ - (void)start self->_surfaceHandler->start(); [self _propagateStageChange]; - [self->_surfacePresenter setupAnimationDriverWithSurfaceHandler:*self->_surfaceHandler]; + // `start()` is a no-op if the surface got unregistered in the meantime (e.g. by an instance teardown), and then + // there is no ShadowTree to take a MountingCoordinator from. + if (self->_surfaceHandler->getStatus() == SurfaceHandler::Status::Running) { + [self->_surfacePresenter setupAnimationDriverWithSurfaceHandler:*self->_surfaceHandler]; + } + self->_startInFlight = false; }); }); } diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp index 930472f096e9..5c9ed2c2db81 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp @@ -8,6 +8,7 @@ #include "SurfaceHandler.h" #include +#include #include #include @@ -37,8 +38,17 @@ Status SurfaceHandler::getStatus() const noexcept { void SurfaceHandler::start() const noexcept { TraceSection s("SurfaceHandler::start"); std::unique_lock lock(linkMutex_); - react_native_assert( - link_.status == Status::Registered && "Surface must be registered."); + // Callers can race into a second start (-[RCTFabricSurface start] checks the + // status two async hops before this runs). Going on would hand + // ShadowTreeRegistry::add() a second tree for the same SurfaceId, which it + // drops, leaving `link_.shadowTree` dangling; an unregistered surface has no + // `link_.uiManager` at all. So this is a no-op rather than an assert. + if (link_.status != Status::Registered) { + LOG(WARNING) + << "SurfaceHandler::start ignored for a surface that is not in Registered state, surfaceId = " + << getSurfaceId() << ", status = " << static_cast(link_.status); + return; + } react_native_assert( getLayoutConstraints().layoutDirection != LayoutDirection::Undefined && "layoutDirection must be set."); From 6905cbcca1c1c27f3f7f9896f38eded84e0dbac9 Mon Sep 17 00:00:00 2001 From: Daewoon Kim Date: Mon, 28 Sep 2026 12:30:24 +0900 Subject: [PATCH 2/3] =?UTF-8?q?fix(scatterlab):=20surface=20start=20?= =?UTF-8?q?=EC=9D=98=20in-flight=20=ED=94=8C=EB=9E=98=EA=B7=B8=EB=A5=BC=20?= =?UTF-8?q?start()=20=EC=A7=81=ED=9B=84=20=EB=82=B4=EB=A6=B0=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_startInFlight` 를 global 블록 끝이 아니라 `SurfaceHandler::start()` 가 돌아온 직후에 내린다. status 는 그 자리에서 한 번 읽어 animation driver 게이트에 그대로 쓴다. start() 가 no-op 이었어도 플래그는 곧바로 내려간다. 블록 끝까지 플래그가 서 있으면 `_propagateStageChange` 와 `setupAnimationDriver` 가 도는 동안 suspend → resume 이 surface 를 stop·재등록하고 부른 `-start` 가 버려진다. 그러면 Registered 인데 ShadowTree 가 없는 빈 surface 가 남는다. 이제 그 창은 getStatus() 한 번 길이다. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WXbb9Sp9jdMsrnBGjzVdgP --- .../react-native/React/Fabric/Surface/RCTFabricSurface.mm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm index 8b8a8da85353..73a822e35b55 100644 --- a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm +++ b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm @@ -112,14 +112,15 @@ - (void)start surfaceId:self->_surfaceHandler->getSurfaceId()]; dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{ self->_surfaceHandler->start(); + auto status = self->_surfaceHandler->getStatus(); + self->_startInFlight = false; [self _propagateStageChange]; // `start()` is a no-op if the surface got unregistered in the meantime (e.g. by an instance teardown), and then // there is no ShadowTree to take a MountingCoordinator from. - if (self->_surfaceHandler->getStatus() == SurfaceHandler::Status::Running) { + if (status == SurfaceHandler::Status::Running) { [self->_surfacePresenter setupAnimationDriverWithSurfaceHandler:*self->_surfaceHandler]; } - self->_startInFlight = false; }); }); } From 8c284f82373848d409c100a5aaf0e363ab502f3c Mon Sep 17 00:00:00 2001 From: Daewoon Kim Date: Mon, 28 Sep 2026 12:57:06 +0900 Subject: [PATCH 3/3] =?UTF-8?q?fix(scatterlab):=20-start=20=EA=B0=80=20in-?= =?UTF-8?q?flight=20=ED=94=8C=EB=9E=98=EA=B7=B8=EB=A5=BC=20status=20?= =?UTF-8?q?=EB=B3=B4=EB=8B=A4=20=EB=A8=BC=EC=A0=80=20=EC=9E=A1=EB=8A=94?= =?UTF-8?q?=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `-start` 는 status 를 읽은 뒤 `_startInFlight.exchange(true)` 를 했다. 둘을 바꾸는 쪽(global 블록의 `start()` 와 플래그 해제)은 `_surfaceMutex` 를 잡지 않는다. 그래서 두 번째 `-start` 가 Registered 를 읽고 밀린 사이 앞선 블록이 끝나면, 그 `-start` 는 false 를 읽고 통과한다. 그러면 `attachSurfaceToView` 가 한 번 더 돌고, 읽어 둔 status 가 Running 이라 animation driver 도 `setMountingOverrideDelegate` 에 한 번 더 들어간다. 플래그를 먼저 잡고 status 를 나중에 본다. exchange 가 false 를 읽었다면 그 값은 앞선 블록의 seq_cst store 이므로, 뒤따르는 `getStatus()` 는 Running 을 보고 반환한다. 반환할 때 되돌리는 store 는 `-start` 만 플래그를 세우므로 다른 블록과 부딪히지 않는다. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WXbb9Sp9jdMsrnBGjzVdgP --- .../react-native/React/Fabric/Surface/RCTFabricSurface.mm | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm index 73a822e35b55..1593f039505b 100644 --- a/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm +++ b/packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm @@ -101,7 +101,12 @@ - (void)start { std::lock_guard lock(_surfaceMutex); - if (_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered || _startInFlight.exchange(true)) { + // Take the flag before reading the status, so a start that finished in between reads as `Running` here. + if (_startInFlight.exchange(true)) { + return; + } + if (_surfaceHandler->getStatus() != SurfaceHandler::Status::Registered) { + _startInFlight = false; return; }