-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: Break circular reference in NativeProxy
and AndroidUIScheduler
#6697
base: main
Are you sure you want to change the base?
fix: Break circular reference in NativeProxy
and AndroidUIScheduler
#6697
Conversation
...c/reactNativeVersionPatch/NativeProxyFabric/latest/com/swmansion/reanimated/NativeProxy.java
Outdated
Show resolved
Hide resolved
...act-native-reanimated/android/src/main/java/com/swmansion/reanimated/AndroidUIScheduler.java
Outdated
Show resolved
Hide resolved
// Module might be already destroyed. | ||
if (!javaPart_) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of checking this every time, but it sure is the best way if we don't want any refactors here. However, I'd much prefer something among the lines for transparency:
// Module might be already destroyed. | |
if (!javaPart_) { | |
return; | |
} | |
if (invalidated) { | |
return; | |
} |
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, the approach is okay 👍
b450de3
to
db174c8
Compare
db174c8
to
a733aa2
Compare
Summary
Breaks circular reference in
NativeProxy
andAndroidUIScheduler
.Needs #6671 to work.
NativeProxy
andAndroidUIScheduler
is aHybridClass
with a C++ counterpart holding a global reference to the Java object. This structure creates a circular reference between C++ and Java, which the garbage collector cannot clean up. To resolve this issue, I manually removed the reference between C++ and Java by resetting the global ref during the invalidation of theNativeProxy
andAndroidUIScheduler
.Test plan
I tested it with a new project using Expo (SDK 52) and with Fabric enabled. I reloaded the app several times and performed a heap dump to verify that everything was removed correctly.