Skip to content

Commit 117655b

Browse files
committed
fix: Memory mismanagement with UI scheduled callbacks (#6900)
The pattern we used with scheduling callbacks on the UI thread - capturing `this` as a reference - is prone to memory issues. Given such code: ```c++ void WorkletsModuleProxy::scheduleOnUI( jsi::Runtime &rt, const jsi::Value &worklet) { auto shareableWorklet = extractShareableOrThrow<ShareableWorklet>( rt, worklet, "[Worklets] Only worklets can be scheduled to run on UI."); uiScheduler_->scheduleOnUI( [this, shareableWorklet] { this->uiWorkletRuntime_->runGuarded(shareableWorklet); }); } ``` We are likely to run into accessing invalidated memory during a reload. This is due to fact that `WorkletsModuleProxy` is managed by some object held by the instance of React Native. Let's look at the following scenario. 1. `WorkletsModuleProxy` is created on the JS thread and held by the `WorkletsModule` Native Module. 2. `WorkletsModuleProxy::scheduleOnUI` is invoked on the JS thread. The callback is scheduled to be executed on the UI thread. 3. Application's reload gets triggered. A tear down of React Native is starting on the JS thread. 4. `WorkletsModule` gets destroyed. Therefore, `WorkletsModuleProxy` is released and also destroyed. 5. The callback is finally executed on the UI thread by the scheduler. However, `this` has been invalidated. The App crashes. Keep in mind that this isn't exclusive to thread jumps exclusively. Calling `scheduleOnUI` on the UI thread could still result in the callback executing after the memory has been invalidated. `WorkletsModuleProxy` is only an example here, the problem could manifest in all the places where we pass lambdas that capture `this` by reference. To fix this I refactored the code so everytime we pass `this` to a scheduled callback, it would be done via a _weak pointer_ which would lock the object and prevent it from being destroyed while the callback is being executed on the UI thread. Perhaps some bits of code don't need this safety measure due to a heuristic existing that guarantees that respective memory won't be invalidated before the callback gets executed. However, I found it extremely challenging and unreliable to come up with these heuristics, as they could possibly break at any future change of the code. - ReanimatedCommitHook - LayoutAnimationProxy - ReanimatedModuleProxy - WorkletsModuleProxy - WorkletRuntime - NativeProxy Reloading the app no longer causes a crash on a scheduled UI callback.
1 parent 530e371 commit 117655b

File tree

3 files changed

+8
-5
lines changed

3 files changed

+8
-5
lines changed

packages/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class ReanimatedModuleProxy : public ReanimatedModuleProxySpec {
5454
jsi::Runtime &rt,
5555
const jsi::Value &workletRuntimeValue,
5656
const jsi::Value &shareableWorkletValue) override;
57-
57+
5858
void invalidate();
5959

6060
jsi::Value registerEventHandler(

packages/react-native-reanimated/Common/cpp/worklets/Tools/SingleInstanceChecker.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,14 @@ SingleInstanceChecker<T>::SingleInstanceChecker() {
5252
std::string className =
5353
__cxxabiv1::__cxa_demangle(typeid(T).name(), nullptr, nullptr, &status);
5454

55+
// React Native can spawn up to two instances of a Native Module at the same
56+
// time. This happens during a reload when a new instance of React Native is
57+
// created while simultaneously the old one is torn down.
58+
instanceCount_++;
5559
assertWithMessage(
56-
instanceCount_ == 0,
57-
"[Reanimated] More than one instance of " + className +
60+
instanceCount_ <= 2,
61+
"[Reanimated] More than two instances of " + className +
5862
" present. This may indicate a memory leak due to a retain cycle.");
59-
60-
instanceCount_++;
6163
}
6264

6365
template <class T>

packages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ void NativeProxy::invalidateCpp() {
612612
if (reanimatedModuleProxy_ != nullptr) {
613613
reanimatedModuleProxy_->invalidate();
614614
}
615+
layoutAnimations_->cthis()->invalidate();
615616
reanimatedModuleProxy_.reset();
616617
}
617618

0 commit comments

Comments
 (0)