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.

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 host obj reference loop #4082

Merged
merged 1 commit into from Feb 21, 2023
Merged

Fix host obj reference loop #4082

merged 1 commit into from Feb 21, 2023

Conversation

kmagiera
Copy link
Member

Summary

Some recent changes (#4060 and #4024) resulted in introducing a reference loop. The reason was that we started capturing the whole module as host object due to the fact we referenced the module in defineAnimation method. As a result, the VM wouldn't deallocate properly and we started getting instance checker crashes on reloads.

Test plan

Start app on iOS. Try out reloading JS several times -> no crash.

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯

@kmagiera kmagiera added this pull request to the merge queue Feb 21, 2023
Merged via the queue into main with commit 71f136b Feb 21, 2023
@kmagiera kmagiera deleted the fix-host-obj-loop branch February 21, 2023 14:40
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

Some recent changes (software-mansion#4060 and software-mansion#4024) resulted in introducing a
reference loop. The reason was that we started capturing the whole
module as host object due to the fact we referenced the module in
`defineAnimation` method. As a result, the VM wouldn't deallocate
properly and we started getting instance checker crashes on reloads.

## Test plan

Start app on iOS. Try out reloading JS several times -> no crash.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants