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

refactor: explicit invalidations for native and cpp #6850

Merged
merged 6 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -844,4 +844,10 @@ void ReanimatedModuleProxy::unsubscribeFromKeyboardEvents(
unsubscribeFromKeyboardEventsFunction_(listenerId.asNumber());
}

void ReanimatedModuleProxy::invalidate() {
// Make sure to release WorkletsModuleProxy on invalidate to allow it
// to destroy its runtime during the invalidation stage.
workletsModuleProxy_.reset();
}

} // namespace reanimated
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class ReanimatedModuleProxy : public ReanimatedModuleProxySpec {

~ReanimatedModuleProxy();

void invalidate();

jsi::Value registerEventHandler(
jsi::Runtime &rt,
const jsi::Value &worklet,
Expand Down Expand Up @@ -175,7 +177,7 @@ class ReanimatedModuleProxy : public ReanimatedModuleProxySpec {

const bool isBridgeless_;
const bool isReducedMotion_;
const std::shared_ptr<WorkletsModuleProxy> workletsModuleProxy_;
std::shared_ptr<WorkletsModuleProxy> workletsModuleProxy_;
const std::string valueUnpackerCode_;

std::unique_ptr<EventHandlerRegistry> eventHandlerRegistry_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@ SingleInstanceChecker<T>::SingleInstanceChecker() {
std::string className =
__cxxabiv1::__cxa_demangle(typeid(T).name(), nullptr, nullptr, &status);

// Only one instance should exist, but it is possible for two instances
// to co-exist during a reload.
assertWithMessage(
instanceCount_ <= 1,
instanceCount_ < 1,
tjzel marked this conversation as resolved.
Show resolved Hide resolved
"[Reanimated] More than one instance of " + className +
" present. This may indicate a memory leak due to a retain cycle.");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ protected HybridData getHybridData() {
return mHybridData;
}

private native void invalidateCpp();

public void invalidate() {
invalidateCpp();
}

public static NativeMethodsHolder createNativeMethodsHolder(
LayoutAnimations ignoredLayoutAnimations) {
return new NativeMethodsHolder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ void NativeProxy::registerNatives() {
makeNativeMethod(
"isAnyHandlerWaitingForEvent",
NativeProxy::isAnyHandlerWaitingForEvent),
makeNativeMethod("performOperations", NativeProxy::performOperations)});
makeNativeMethod("performOperations", NativeProxy::performOperations),
makeNativeMethod("invalidateCpp", NativeProxy::invalidateCpp)});
}

void NativeProxy::requestRender(
Expand Down Expand Up @@ -619,4 +620,10 @@ void NativeProxy::setupLayoutAnimations() {
});
}

void NativeProxy::invalidateCpp() {
workletsModuleProxy_.reset();
reanimatedModuleProxy_->invalidate();
reanimatedModuleProxy_.reset();
}

} // namespace reanimated
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ class NativeProxy : public jni::HybridClass<NativeProxy> {
void commonInit(jni::alias_ref<facebook::react::JFabricUIManager::javaobject>
&fabricUIManager);
#endif // RCT_NEW_ARCH_ENABLED

void invalidateCpp();
};

} // namespace reanimated
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ using namespace facebook;
using namespace react;

WorkletsModule::WorkletsModule(
jni::alias_ref<WorkletsModule::javaobject> jThis,
jsi::Runtime *rnRuntime,
const std::string &valueUnpackerCode,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
const std::shared_ptr<facebook::react::CallInvoker> &jsCallInvoker,
const std::shared_ptr<worklets::JSScheduler> &jsScheduler,
const std::shared_ptr<UIScheduler> &uiScheduler)
: javaPart_(jni::make_global(jThis)),
rnRuntime_(rnRuntime),
: rnRuntime_(rnRuntime),
workletsModuleProxy_(std::make_shared<WorkletsModuleProxy>(
*rnRuntime,
valueUnpackerCode,
Expand All @@ -38,7 +36,7 @@ WorkletsModule::WorkletsModule(
}

jni::local_ref<WorkletsModule::jhybriddata> WorkletsModule::initHybrid(
jni::alias_ref<jhybridobject> jThis,
jni::alias_ref<jhybridobject> /*jThis*/,
jlong jsContext,
const std::string &valueUnpackerCode,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
Expand All @@ -52,7 +50,6 @@ jni::local_ref<WorkletsModule::jhybriddata> WorkletsModule::initHybrid(
std::make_shared<worklets::JSScheduler>(*rnRuntime, jsCallInvoker);
auto uiScheduler = androidUIScheduler->cthis()->getUIScheduler();
return makeCxxInstance(
jThis,
rnRuntime,
tjzel marked this conversation as resolved.
Show resolved Hide resolved
valueUnpackerCode,
messageQueueThread,
Expand All @@ -61,8 +58,14 @@ jni::local_ref<WorkletsModule::jhybriddata> WorkletsModule::initHybrid(
uiScheduler);
}

void WorkletsModule::invalidateCpp() {
workletsModuleProxy_.reset();
}

void WorkletsModule::registerNatives() {
registerHybrid({makeNativeMethod("initHybrid", WorkletsModule::initHybrid)});
registerHybrid(
{makeNativeMethod("initHybrid", WorkletsModule::initHybrid),
makeNativeMethod("invalidateCpp", WorkletsModule::invalidateCpp)});
}

} // namespace worklets
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class WorkletsModule : public jni::HybridClass<WorkletsModule> {
"Lcom/swmansion/worklets/WorkletsModule;";

static jni::local_ref<jhybriddata> initHybrid(
jni::alias_ref<jhybridobject> jThis,
jni::alias_ref<jhybridobject> /*jThis*/,
jlong jsContext,
const std::string &valueUnpackerCode,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
Expand All @@ -46,19 +46,19 @@ class WorkletsModule : public jni::HybridClass<WorkletsModule> {
}

private:
friend HybridBase;
jni::global_ref<WorkletsModule::javaobject> javaPart_;
jsi::Runtime *rnRuntime_;
std::shared_ptr<WorkletsModuleProxy> workletsModuleProxy_;

explicit WorkletsModule(
jni::alias_ref<WorkletsModule::jhybridobject> jThis,
tomekzaw marked this conversation as resolved.
Show resolved Hide resolved
jsi::Runtime *rnRuntime,
tjzel marked this conversation as resolved.
Show resolved Hide resolved
const std::string &valueUnpackerCode,
jni::alias_ref<JavaMessageQueueThread::javaobject> messageQueueThread,
const std::shared_ptr<facebook::react::CallInvoker> &jsCallInvoker,
const std::shared_ptr<worklets::JSScheduler> &jsScheduler,
const std::shared_ptr<UIScheduler> &uiScheduler);

void invalidateCpp();

friend HybridBase;
jsi::Runtime *rnRuntime_;
tomekzaw marked this conversation as resolved.
Show resolved Hide resolved
std::shared_ptr<WorkletsModuleProxy> workletsModuleProxy_;
};

} // namespace worklets
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ public void invalidate() {
}

if (mNativeProxy != null) {
mNativeProxy.invalidate();
mNativeProxy = null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ public boolean installTurboModule(String valueUnpackerCode) {
}

public void invalidate() {
// We have to destroy extra runtimes when invalidate is called. If we clean
// it up later instead there's a chance the runtime will retain references
// to invalidated memory and will crash on its destruction.
invalidateCpp();
mAndroidUIScheduler.deactivate();
tjzel marked this conversation as resolved.
Show resolved Hide resolved
}

private native void invalidateCpp();
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ protected HybridData getHybridData() {
return mHybridData;
}

private native void invalidateCpp();

public void invalidate() {
invalidateCpp();
}

public static NativeMethodsHolder createNativeMethodsHolder(LayoutAnimations layoutAnimations) {
WeakReference<LayoutAnimations> weakLayoutAnimations = new WeakReference<>(layoutAnimations);
return new NativeMethodsHolder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,13 @@ @implementation WorkletsModule {
return @YES;
}

- (void)invalidate
{
// We have to destroy extra runtimes when invalidate is called. If we clean
// it up later instead there's a chance the runtime will retain references
// to invalidated memory and will crash on destruction.
workletsModuleProxy_.reset();
[super invalidate];
tjzel marked this conversation as resolved.
Show resolved Hide resolved
}

@end
Loading