Skip to content

Fix unsafe rawPointer access in cloneMultiple#55613

Closed
bartlomiejbloniarz wants to merge 1 commit intomainfrom
export-D93596770
Closed

Fix unsafe rawPointer access in cloneMultiple#55613
bartlomiejbloniarz wants to merge 1 commit intomainfrom
export-D93596770

Conversation

@bartlomiejbloniarz
Copy link
Copy Markdown
Contributor

@bartlomiejbloniarz bartlomiejbloniarz commented Feb 18, 2026

Summary:
The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This cloneMultiple method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Changelog:
[General][Breaking] - fix unsafe rawPointer access in cloneMultiple.

Differential Revision: D93596770

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 18, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync bot commented Feb 18, 2026

@bartlomiejbloniarz has exported this pull request. If you are a Meta employee, you can view the originating Diff in D93596770.

@facebook-github-bot facebook-github-bot added the p: Facebook Partner: Facebook label Feb 18, 2026
meta-codesync bot pushed a commit that referenced this pull request Mar 3, 2026
Summary:

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@meta-codesync meta-codesync bot force-pushed the export-D93596770 branch from 5ecac9d to 37d37ce Compare March 3, 2026 12:00
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: build_android
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github publishAllToMavenTempLocal build 9.3.1 Build Scan not published

bartlomiejbloniarz pushed a commit that referenced this pull request Mar 3, 2026
Summary:
Pull Request resolved: #55613

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @coado in 8e5de46

When will my fix make it into a release? | How to file a pick request?

@react-native-bot react-native-bot added the Merged This PR has been merged. label Mar 3, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: build_android
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github publishAllToMavenTempLocal build 9.3.1 Build Scan not published

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: run_fantom_tests
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github :private:react-native-fantom:buildFantomTester 9.3.1 Build Scan not published

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: run_fantom_tests
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github :private:react-native-fantom:buildFantomTester 9.3.1 Build Scan not published

meta-codesync bot pushed a commit that referenced this pull request Mar 3, 2026
Summary:

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@meta-codesync meta-codesync bot force-pushed the export-D93596770 branch from 8e5de46 to dfffda3 Compare March 3, 2026 12:27
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: build_android
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github publishAllToMavenTempLocal build 9.3.1 Build Scan not published

bartlomiejbloniarz pushed a commit that referenced this pull request Mar 3, 2026
Summary:
Pull Request resolved: #55613

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @coado in f102dbe

When will my fix make it into a release? | How to file a pick request?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: build_android
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github publishAllToMavenTempLocal build 9.3.1 Build Scan not published

bartlomiejbloniarz pushed a commit that referenced this pull request Mar 3, 2026
Summary:
Pull Request resolved: #55613

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @coado in 6675f95

When will my fix make it into a release? | How to file a pick request?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: build_android
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github publishAllToMavenTempLocal build 9.3.1 Build Scan not published

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: run_fantom_tests
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github :private:react-native-fantom:buildFantomTester 9.3.1 Build Scan not published

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: run_fantom_tests
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github :private:react-native-fantom:buildFantomTester 9.3.1 Build Scan not published

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Job Summary for Gradle

Test All :: run_fantom_tests
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
react-native-github :private:react-native-fantom:buildFantomTester 9.3.1 Build Scan not published

meta-codesync bot pushed a commit that referenced this pull request Mar 3, 2026
Summary:

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@meta-codesync meta-codesync bot force-pushed the export-D93596770 branch from 6675f95 to c66970c Compare March 3, 2026 14:08
bartlomiejbloniarz pushed a commit that referenced this pull request Mar 3, 2026
Summary:
Pull Request resolved: #55613

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @coado in 639c289

When will my fix make it into a release? | How to file a pick request?

Summary:
Pull Request resolved: #55613

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Reviewed By: zeyap, javache

Differential Revision: D93596770
@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @coado in 41f59c4

When will my fix make it into a release? | How to file a pick request?

@meta-codesync meta-codesync bot closed this in 1d47693 Mar 4, 2026
@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @coado in 1d47693

When will my fix make it into a release? | How to file a pick request?

alanleedev pushed a commit that referenced this pull request Mar 9, 2026
Summary:
Pull Request resolved: #55613

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Changelog:
[General][Breaking] - fix unsafe rawPointer access in cloneMultiple.

Reviewed By: zeyap, javache

Differential Revision: D93596770

fbshipit-source-id: c4d99b51875968ebce50358c19502cba02c50685
zoontek pushed a commit to zoontek/react-native that referenced this pull request Mar 9, 2026
Summary:
Pull Request resolved: facebook#55613

The cloneMultiple method was written in a way to accept a list of families that are presumed to be owned by the api caller. This designe was mostly aimed at reanimated, that holds refernces to ShadowNodes (that own these families). In the case of AnimationBackend this didn't work properly, as when the view is unmounted we would lose the ShadowNodeFamily shared_ptr that we hold and it could get deallocated.

Since now we can get an owning reference to ShadowNodeFamily from ShadowNode::getFamilyShared, we don't have to keep this old unsafe api. Instead we require the caller to have an owning reference with the api itself.

This `cloneMultiple` method isn't really adopted in the community, so the breaking change shouldn't be a big problem.

Changelog:
[General][Breaking] - fix unsafe rawPointer access in cloneMultiple.

Reviewed By: zeyap, javache

Differential Revision: D93596770

fbshipit-source-id: c4d99b51875968ebce50358c19502cba02c50685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. meta-exported p: Facebook Partner: Facebook p: Software Mansion Partner: Software Mansion Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants