Skip to content

Commit 5688f32

Browse files
Bartlomiej Bloniarzfacebook-github-bot
authored andcommitted
Fix SIGSEGV in animation backend callback by capturing weak_ptr
Summary: 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. Differential Revision: D97101348
1 parent 8101fc7 commit 5688f32

2 files changed

Lines changed: 8 additions & 4 deletions

File tree

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)