Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Memory mismanagement with UI scheduled callbacks #6900

Merged
merged 15 commits into from
Jan 23, 2025
Merged

fix: Memory mismanagement with UI scheduled callbacks #6900

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b3494e4
fix: memory management for lambda this capturing
tjzel Jan 14, 2025
d454c3d
chore: add more weak captures
tjzel Jan 16, 2025
0dda87c
chore: apply formatting
tjzel Jan 16, 2025
cf8aa1d
fix: setting up this referencing callbacks
tjzel Jan 16, 2025
f0a5bab
chore: fix typo in comment
tjzel Jan 20, 2025
ce9c8b8
Merge branch 'main' into @tjzel/fix-threading-memory-issues
tjzel Jan 22, 2025
ca2092d
fix: css memory leaks
tjzel Jan 22, 2025
171079c
fix: css memory leaks
tjzel Jan 22, 2025
896409f
chore: move to weakThis strongThis convention
tjzel Jan 22, 2025
cf96d67
fix: missing UI schedule
tjzel Jan 22, 2025
a911fa4
chore: shared_ptr capture for synchronouslt executed callback in Rean…
tjzel Jan 23, 2025
f40898f
Merge branch 'main' into @tjzel/fix-threading-memory-issues
tjzel Jan 23, 2025
9f41fe0
chore: apply new formatting
tjzel Jan 23, 2025
8b7e597
chore: apply new formatting
tjzel Jan 23, 2025
84cee56
Merge branch 'main' into @tjzel/fix-threading-memory-issues
tjzel Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions apps/fabric-example/ios/FabricExample.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@
};
1232BDDFC9DE5BCE2776187D /* [CP-User] Generate metadata for clangd */ = {
isa = PBXShellScriptBuildPhase;
alwaysOutOfDate = 1;
alwaysOutOfDate = true;
buildActionMask = 2147483647;
files = (
);
Expand Down Expand Up @@ -573,7 +573,10 @@
"-DFOLLY_CFG_NO_COROUTINES=1",
"-DFOLLY_HAVE_CLOCK_GETTIME=1",
);
OTHER_LDFLAGS = "$(inherited) ";
OTHER_LDFLAGS = (
"$(inherited)",
" ",
);
REACT_NATIVE_PATH = "${PODS_ROOT}/../../../../node_modules/react-native";
SDKROOT = iphoneos;
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "$(inherited) DEBUG";
Expand Down Expand Up @@ -646,7 +649,10 @@
"-DFOLLY_CFG_NO_COROUTINES=1",
"-DFOLLY_HAVE_CLOCK_GETTIME=1",
);
OTHER_LDFLAGS = "$(inherited) ";
OTHER_LDFLAGS = (
"$(inherited)",
" ",
);
REACT_NATIVE_PATH = "${PODS_ROOT}/../../../../node_modules/react-native";
SDKROOT = iphoneos;
USE_HERMES = true;
Expand Down
2 changes: 1 addition & 1 deletion apps/fabric-example/ios/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2229,7 +2229,7 @@ SPEC CHECKSUMS:
RNScreens: d0854539b51a53e38b61bcc9fb402439a9c73b26
RNSVG: 7e38044415125a1d108294377de261d2fe2c54c9
SocketRocket: d4aabe649be1e368d1318fdf28a022d714d65748
Yoga: dd05f79b1943c96893294ee5040576aa6e8a8d82
Yoga: 0c8754b0ea9edb13b6ce6b60f0f69eb5f164f16a

PODFILE CHECKSUM: 4a9e0af2552a3fcd2303b56ad75e373f8bae65b9

Expand Down
2 changes: 1 addition & 1 deletion apps/macos-example/macos/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1868,7 +1868,7 @@ SPEC CHECKSUMS:
RNReanimated: ed490424d3b8b9f2acd104577c73b374fc79310b
RNSVG: 669ed128ab9005090c612a0d627dbecb6ab5c76f
SocketRocket: 9ee265c4b5ae2382d18e4ee1d2dd2d7af0ff1ab5
Yoga: 209f62622a01344dbb9fa8d348610eaeb7df2cca
Yoga: 446e6f351a519539ff00a1159fe41e589aab1b94

PODFILE CHECKSUM: 8d50cc2acc9f6a6b1a12bd9106b86385ad72266f

Expand Down
2 changes: 1 addition & 1 deletion apps/paper-example/ios/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2060,7 +2060,7 @@ SPEC CHECKSUMS:
RNScreens: 81237378a6d7921fb2dbb6a6c6c45c3d8cc696bb
RNSVG: 515a902fc18a375907eb4c3abec0b803fbfa37ef
SocketRocket: d4aabe649be1e368d1318fdf28a022d714d65748
Yoga: dd05f79b1943c96893294ee5040576aa6e8a8d82
Yoga: 0c8754b0ea9edb13b6ce6b60f0f69eb5f164f16a

PODFILE CHECKSUM: f6c84e0ec8eddea6d3ee15329987727bd1e6ff08

Expand Down
12 changes: 9 additions & 3 deletions apps/paper-example/ios/ReanimatedExample.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@
};
EC2A315BF2310B4CCE81E020 /* [CP-User] Generate metadata for clangd */ = {
isa = PBXShellScriptBuildPhase;
alwaysOutOfDate = 1;
alwaysOutOfDate = true;
buildActionMask = 2147483647;
files = (
);
Expand Down Expand Up @@ -579,7 +579,10 @@
"-DFOLLY_CFG_NO_COROUTINES=1",
"-DFOLLY_HAVE_CLOCK_GETTIME=1",
);
OTHER_LDFLAGS = "$(inherited) ";
OTHER_LDFLAGS = (
"$(inherited)",
" ",
);
REACT_NATIVE_PATH = "${PODS_ROOT}/../../../../node_modules/react-native";
SDKROOT = iphoneos;
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "$(inherited) DEBUG";
Expand Down Expand Up @@ -652,7 +655,10 @@
"-DFOLLY_CFG_NO_COROUTINES=1",
"-DFOLLY_HAVE_CLOCK_GETTIME=1",
);
OTHER_LDFLAGS = "$(inherited) ";
OTHER_LDFLAGS = (
"$(inherited)",
" ",
);
REACT_NATIVE_PATH = "${PODS_ROOT}/../../../../node_modules/react-native";
SDKROOT = iphoneos;
USE_HERMES = true;
Expand Down
2 changes: 1 addition & 1 deletion apps/tvos-example/ios/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1888,7 +1888,7 @@ SPEC CHECKSUMS:
ReactCommon: b927fd46115bd2acb146e24cf1a08f22abda8b3f
RNReanimated: 3a5e1e235c940894097b0734aad9ebce45431ddd
SocketRocket: d4aabe649be1e368d1318fdf28a022d714d65748
Yoga: 2b0e5affb9ab46e4ebad33530df829c153c323d8
Yoga: 651e5fd560c7e408ab9d9ca44b8de1b622d7f0cc

PODFILE CHECKSUM: 79e1477a8eb76b717bdd7c1610f7f8e6772536a9

Expand Down
13 changes: 10 additions & 3 deletions packages/react-native-reanimated/Common/cpp/reanimated/Fabric/ReanimatedCommitHook.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,21 @@ void ReanimatedCommitHook::maybeInitializeLayoutAnimations(
// when a new surfaceId is observed we call setMountingOverrideDelegate
// for all yet unseen surfaces
uiManager_->getShadowTreeRegistry().enumerate(
[this](const ShadowTree &shadowTree, bool &stop) {
if (shadowTree.getSurfaceId() <= currentMaxSurfaceId_) {
[weakReanimatedCommitHook = weak_from_this()](
tjzel marked this conversation as resolved.
Show resolved Hide resolved
const ShadowTree &shadowTree, bool &stop) {
auto reanimatedCommitHook = weakReanimatedCommitHook.lock();
if (!reanimatedCommitHook) {
return;
}

if (shadowTree.getSurfaceId() <=
reanimatedCommitHook->currentMaxSurfaceId_) {
// the set function actually adds our delegate to a list, so we
// shouldn't invoke it twice for the same surface
return;
}
shadowTree.getMountingCoordinator()->setMountingOverrideDelegate(
layoutAnimationsProxy_);
reanimatedCommitHook->layoutAnimationsProxy_);
});
currentMaxSurfaceId_ = surfaceId;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/react-native-reanimated/Common/cpp/reanimated/Fabric/ReanimatedCommitHook.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ using namespace facebook::react;

namespace reanimated {

class ReanimatedCommitHook : public UIManagerCommitHook {
class ReanimatedCommitHook
: public UIManagerCommitHook,
public std::enable_shared_from_this<ReanimatedCommitHook> {
public:
ReanimatedCommitHook(
const std::shared_ptr<PropsRegistry> &propsRegistry,
Expand Down
165 changes: 104 additions & 61 deletions .../react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,30 +618,42 @@ void LayoutAnimationsProxy::startEnteringAnimation(
static_cast<const ViewProps &>(*mutation.newChildShadowView.props);
auto opacity = viewProps.opacity;

uiScheduler_->scheduleOnUI(
[finalView, current, parent, mutation, opacity, this, tag]() {
Rect window{};
{
auto lock = std::unique_lock<std::recursive_mutex>(mutex);
layoutAnimations_.insert_or_assign(
tag, LayoutAnimation{finalView, current, parent, opacity});
window =
surfaceManager.getWindow(mutation.newChildShadowView.surfaceId);
}
uiScheduler_->scheduleOnUI([weakLayoutAnimationsProxy = weak_from_this(),
finalView,
current,
parent,
mutation,
opacity,
tag]() {
auto layoutAnimationsProxy = weakLayoutAnimationsProxy.lock();
if (!layoutAnimationsProxy) {
return;
}

Snapshot values(mutation.newChildShadowView, window);
jsi::Object yogaValues(uiRuntime_);
yogaValues.setProperty(uiRuntime_, "targetOriginX", values.x);
yogaValues.setProperty(uiRuntime_, "targetGlobalOriginX", values.x);
yogaValues.setProperty(uiRuntime_, "targetOriginY", values.y);
yogaValues.setProperty(uiRuntime_, "targetGlobalOriginY", values.y);
yogaValues.setProperty(uiRuntime_, "targetWidth", values.width);
yogaValues.setProperty(uiRuntime_, "targetHeight", values.height);
yogaValues.setProperty(uiRuntime_, "windowWidth", values.windowWidth);
yogaValues.setProperty(uiRuntime_, "windowHeight", values.windowHeight);
layoutAnimationsManager_->startLayoutAnimation(
uiRuntime_, tag, LayoutAnimationType::ENTERING, yogaValues);
});
Rect window{};
{
auto &mutex = layoutAnimationsProxy->mutex;
auto lock = std::unique_lock<std::recursive_mutex>(mutex);
layoutAnimationsProxy->layoutAnimations_.insert_or_assign(
tag, LayoutAnimation{finalView, current, parent, opacity});
window = layoutAnimationsProxy->surfaceManager.getWindow(
mutation.newChildShadowView.surfaceId);
}

Snapshot values(mutation.newChildShadowView, window);
auto &uiRuntime = layoutAnimationsProxy->uiRuntime_;
jsi::Object yogaValues(uiRuntime);
yogaValues.setProperty(uiRuntime, "targetOriginX", values.x);
yogaValues.setProperty(uiRuntime, "targetGlobalOriginX", values.x);
yogaValues.setProperty(uiRuntime, "targetOriginY", values.y);
yogaValues.setProperty(uiRuntime, "targetGlobalOriginY", values.y);
yogaValues.setProperty(uiRuntime, "targetWidth", values.width);
yogaValues.setProperty(uiRuntime, "targetHeight", values.height);
yogaValues.setProperty(uiRuntime, "windowWidth", values.windowWidth);
yogaValues.setProperty(uiRuntime, "windowHeight", values.windowHeight);
layoutAnimationsProxy->layoutAnimationsManager_->startLayoutAnimation(
uiRuntime, tag, LayoutAnimationType::ENTERING, yogaValues);
});
}

void LayoutAnimationsProxy::startExitingAnimation(
Expand All @@ -652,29 +664,41 @@ void LayoutAnimationsProxy::startExitingAnimation(
#endif
auto surfaceId = mutation.oldChildShadowView.surfaceId;

uiScheduler_->scheduleOnUI([this, tag, mutation, surfaceId]() {
uiScheduler_->scheduleOnUI([weakLayoutAnimationsProxy = weak_from_this(),
tag,
mutation,
surfaceId]() {
auto layoutAnimationsProxy = weakLayoutAnimationsProxy.lock();
if (!layoutAnimationsProxy) {
return;
}

auto oldView = mutation.oldChildShadowView;
Rect window{};
{
auto &mutex = layoutAnimationsProxy->mutex;
auto lock = std::unique_lock<std::recursive_mutex>(mutex);
createLayoutAnimation(mutation, oldView, surfaceId, tag);
window = surfaceManager.getWindow(surfaceId);
layoutAnimationsProxy->createLayoutAnimation(
mutation, oldView, surfaceId, tag);
window = layoutAnimationsProxy->surfaceManager.getWindow(surfaceId);
}

Snapshot values(oldView, window);

jsi::Object yogaValues(uiRuntime_);
yogaValues.setProperty(uiRuntime_, "currentOriginX", values.x);
yogaValues.setProperty(uiRuntime_, "currentGlobalOriginX", values.x);
yogaValues.setProperty(uiRuntime_, "currentOriginY", values.y);
yogaValues.setProperty(uiRuntime_, "currentGlobalOriginY", values.y);
yogaValues.setProperty(uiRuntime_, "currentWidth", values.width);
yogaValues.setProperty(uiRuntime_, "currentHeight", values.height);
yogaValues.setProperty(uiRuntime_, "windowWidth", values.windowWidth);
yogaValues.setProperty(uiRuntime_, "windowHeight", values.windowHeight);
layoutAnimationsManager_->startLayoutAnimation(
uiRuntime_, tag, LayoutAnimationType::EXITING, yogaValues);
layoutAnimationsManager_->clearLayoutAnimationConfig(tag);
auto &uiRuntime = layoutAnimationsProxy->uiRuntime_;
jsi::Object yogaValues(uiRuntime);
yogaValues.setProperty(uiRuntime, "currentOriginX", values.x);
yogaValues.setProperty(uiRuntime, "currentGlobalOriginX", values.x);
yogaValues.setProperty(uiRuntime, "currentOriginY", values.y);
yogaValues.setProperty(uiRuntime, "currentGlobalOriginY", values.y);
yogaValues.setProperty(uiRuntime, "currentWidth", values.width);
yogaValues.setProperty(uiRuntime, "currentHeight", values.height);
yogaValues.setProperty(uiRuntime, "windowWidth", values.windowWidth);
yogaValues.setProperty(uiRuntime, "windowHeight", values.windowHeight);
layoutAnimationsProxy->layoutAnimationsManager_->startLayoutAnimation(
uiRuntime, tag, LayoutAnimationType::EXITING, yogaValues);
layoutAnimationsProxy->layoutAnimationsManager_->clearLayoutAnimationConfig(
tag);
});
}

Expand All @@ -686,36 +710,47 @@ void LayoutAnimationsProxy::startLayoutAnimation(
#endif
auto surfaceId = mutation.oldChildShadowView.surfaceId;

uiScheduler_->scheduleOnUI([this, mutation, surfaceId, tag]() {
uiScheduler_->scheduleOnUI([weakLayoutAnimationsProxy = weak_from_this(),
mutation,
surfaceId,
tag]() {
auto layoutAnimationsProxy = weakLayoutAnimationsProxy.lock();
if (!layoutAnimationsProxy) {
return;
}

auto oldView = mutation.oldChildShadowView;
Rect window{};
{
auto &mutex = layoutAnimationsProxy->mutex;
auto lock = std::unique_lock<std::recursive_mutex>(mutex);
createLayoutAnimation(mutation, oldView, surfaceId, tag);
window = surfaceManager.getWindow(surfaceId);
layoutAnimationsProxy->createLayoutAnimation(
mutation, oldView, surfaceId, tag);
window = layoutAnimationsProxy->surfaceManager.getWindow(surfaceId);
}

Snapshot currentValues(oldView, window);
Snapshot targetValues(mutation.newChildShadowView, window);

jsi::Object yogaValues(uiRuntime_);
yogaValues.setProperty(uiRuntime_, "currentOriginX", currentValues.x);
yogaValues.setProperty(uiRuntime_, "currentGlobalOriginX", currentValues.x);
yogaValues.setProperty(uiRuntime_, "currentOriginY", currentValues.y);
yogaValues.setProperty(uiRuntime_, "currentGlobalOriginY", currentValues.y);
yogaValues.setProperty(uiRuntime_, "currentWidth", currentValues.width);
yogaValues.setProperty(uiRuntime_, "currentHeight", currentValues.height);
yogaValues.setProperty(uiRuntime_, "targetOriginX", targetValues.x);
yogaValues.setProperty(uiRuntime_, "targetGlobalOriginX", targetValues.x);
yogaValues.setProperty(uiRuntime_, "targetOriginY", targetValues.y);
yogaValues.setProperty(uiRuntime_, "targetGlobalOriginY", targetValues.y);
yogaValues.setProperty(uiRuntime_, "targetWidth", targetValues.width);
yogaValues.setProperty(uiRuntime_, "targetHeight", targetValues.height);
yogaValues.setProperty(uiRuntime_, "windowWidth", targetValues.windowWidth);
auto &uiRuntime = layoutAnimationsProxy->uiRuntime_;
jsi::Object yogaValues(uiRuntime);
yogaValues.setProperty(uiRuntime, "currentOriginX", currentValues.x);
yogaValues.setProperty(uiRuntime, "currentGlobalOriginX", currentValues.x);
yogaValues.setProperty(uiRuntime, "currentOriginY", currentValues.y);
yogaValues.setProperty(uiRuntime, "currentGlobalOriginY", currentValues.y);
yogaValues.setProperty(uiRuntime, "currentWidth", currentValues.width);
yogaValues.setProperty(uiRuntime, "currentHeight", currentValues.height);
yogaValues.setProperty(uiRuntime, "targetOriginX", targetValues.x);
yogaValues.setProperty(uiRuntime, "targetGlobalOriginX", targetValues.x);
yogaValues.setProperty(uiRuntime, "targetOriginY", targetValues.y);
yogaValues.setProperty(uiRuntime, "targetGlobalOriginY", targetValues.y);
yogaValues.setProperty(uiRuntime, "targetWidth", targetValues.width);
yogaValues.setProperty(uiRuntime, "targetHeight", targetValues.height);
yogaValues.setProperty(uiRuntime, "windowWidth", targetValues.windowWidth);
yogaValues.setProperty(
uiRuntime_, "windowHeight", targetValues.windowHeight);
layoutAnimationsManager_->startLayoutAnimation(
uiRuntime_, tag, LayoutAnimationType::LAYOUT, yogaValues);
uiRuntime, "windowHeight", targetValues.windowHeight);
layoutAnimationsProxy->layoutAnimationsManager_->startLayoutAnimation(
uiRuntime, tag, LayoutAnimationType::LAYOUT, yogaValues);
});
}

Expand All @@ -731,9 +766,17 @@ void LayoutAnimationsProxy::maybeCancelAnimation(const int tag) const {
return;
}
layoutAnimations_.erase(tag);
uiScheduler_->scheduleOnUI([this, tag]() {
layoutAnimationsManager_->cancelLayoutAnimation(uiRuntime_, tag);
});
uiScheduler_->scheduleOnUI(
[weakLayoutAnimationsProxy = weak_from_this(), tag]() {
auto layoutAnimationsProxy = weakLayoutAnimationsProxy.lock();
if (!layoutAnimationsProxy) {
return;
}

auto &uiRuntime = layoutAnimationsProxy->uiRuntime_;
layoutAnimationsProxy->layoutAnimationsManager_->cancelLayoutAnimation(
uiRuntime, tag);
});
}

void LayoutAnimationsProxy::transferConfigFromNativeID(
Expand Down
4 changes: 3 additions & 1 deletion ...es/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ struct LayoutAnimation {
LayoutAnimation &operator=(const LayoutAnimation &other) = default;
};

struct LayoutAnimationsProxy : public MountingOverrideDelegate {
struct LayoutAnimationsProxy
: public MountingOverrideDelegate,
public std::enable_shared_from_this<LayoutAnimationsProxy> {
mutable std::unordered_map<Tag, std::shared_ptr<Node>> nodeForTag_;
mutable std::unordered_map<Tag, LayoutAnimation> layoutAnimations_;
mutable std::recursive_mutex mutex;
Expand Down
Loading
Loading