Skip to content

Commit 42d98b5

Browse files
Bartlomiej Bloniarzmeta-codesync[bot]
authored andcommitted
Fix SIGSEGV in animation backend callback by capturing weak_ptr (#56130)
Summary: Pull Request resolved: #56130 The NativeAnimatedNodesManager crashes with SIGSEGV (for example in mid 2e1610331cf5e285e285a2af191fe6f3, but there are also others with similar stacks). When the manager is destroyed on the JS thread while the Choreographer thread is still firing animation frames, `pullAnimationMutations` runs on a freed object, because we captured it as a raw `this` pointer. To fix this I added a `weak_from_this` call there, to ensure the manager is still around. One thing about this issue is that it looks like it should also happen for c++ Animated without the Animation Backend, as right after this call we have: ``` if (startOnRenderCallback_) { startOnRenderCallback_([this]() { onRender(); }, isAsync); stopRenderCallbackIfNeeded(false); } } ``` which also captures `this`. On android it is not a problem though, as Android doesn't use `startOnRenderCallback_` (it only relies on `driveCxxAnimations` which is safe). On iOS it seems to be safe due to D91236980, as it adds locking a pointer to the `_nativeAnimatedNodesManagerProvider` (which owns `nativeAnimatedNodesManager`) before calling the scheduled callback. ## Changelog: [General] [Added] - Add enable_shared_from_this to `NativeAnimatedNodesManager` [General] [Changed] - Use weak_ptr when calling pullAnimationMutations in a callback scheduled with the Animation Backend Reviewed By: zeyap Differential Revision: D97101348 fbshipit-source-id: acdad4e689d72b73a945b49907350c68a67729ab
1 parent 0b1ce70 commit 42d98b5

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -552,9 +552,13 @@ void NativeAnimatedNodesManager::startRenderCallbackIfNeeded(bool isAsync) {
552552

553553
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
554554
if (auto animationBackend = animationBackend_.lock()) {
555-
animationBackendCallbackId_ =
556-
animationBackend->start([this](AnimationTimestamp timestamp) {
557-
return pullAnimationMutations(timestamp);
555+
auto weak = weak_from_this();
556+
animationBackendCallbackId_ = animationBackend->start(
557+
[weak](AnimationTimestamp timestamp) -> AnimationMutations {
558+
if (auto self = weak.lock()) {
559+
return self->pullAnimationMutations(timestamp);
560+
}
561+
return {};
558562
});
559563
}
560564

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ using AnimationEndCallback = AsyncCallback<EndResult>;
5151
template <>
5252
struct Bridging<EndResult> : NativeAnimatedTurboModuleEndResultBridging<EndResult> {};
5353

54-
class NativeAnimatedNodesManager {
54+
class NativeAnimatedNodesManager : public std::enable_shared_from_this<NativeAnimatedNodesManager> {
5555
public:
5656
using DirectManipulationCallback = std::function<void(Tag, const folly::dynamic &)>;
5757
using FabricCommitCallback = std::function<void(std::unordered_map<Tag, folly::dynamic> &)>;

0 commit comments

Comments
 (0)