release: 3.17.0 #7071
Merged
release: 3.17.0 #7071
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tjzel
force-pushed
the
@tjzel/release/3.17.0
branch
from
91137be to
0bacccc
Compare
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.
tjzel
force-pushed
the
@tjzel/release/3.17.0
branch
from
0bacccc to
a51df32
Compare
piaskowyk
approved these changes
Feb 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously I only partially cherry-picked #6900, but after more testing of reloads on Android it turned out I had to cherry-pick all of it.