Skip to content

Commit

Permalink
Fix crash due to concurrent writes to frameCallbacks vector (#3859)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes crashes related to concurrent writes to a non-thread-safe
frameCallbacks vector. The primary role of frameCallbacks vector was to
facilitate requestAnimationFrame calls and hence it has not been
designed to allow for other than the main thread to append their
callbacks. However, after recent rewrite #3722 we introduced a new
methods "scheduleOnUI" that was originally meant to interface with a
thread-safe scheduler API but was later updated (see
c8a77da) to use frameCallbacks in order
for errors to be handled using the same code-path the rAF uses. That
change introduces a bug in which we'd access and modify that vector from
different threads which is undesirable. This PR reverts that change and
since #3846 provides a better way of handling JS-errors there is no
longer need for scheduleOnUI callbacks to go throught the
requestAnimationFrame codepath.

## Test plan

The easiest way to reproduce the crash we could find was by using
BokehExample.tsx on Android. This would normally result in a crash after
a couple of seconds of running. With the increased number of circles in
that example (e.g. 400) the crash would be almost immediate. This change
was tested on that example with 400 circles and the crash would not
appear even after some time after launch (few minutes).

Co-authored-by: Tomek Zawadzki <tomasz.zawadzki@swmansion.com>
Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>
Co-authored-by: Juliusz Wajgelt <49338439+jwajgelt@users.noreply.github.com>
  • Loading branch information
4 people authored Dec 15, 2022
1 parent 4707481 commit f8f7b2e
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions Common/cpp/NativeModules/NativeReanimatedModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,11 @@ void NativeReanimatedModule::scheduleOnUI(
assert(
shareableWorklet->valueType() == Shareable::WorkletType &&
"only worklets can be scheduled to run on UI");
frameCallbacks.push_back([=](double timestamp) {
scheduler->scheduleOnUI([=] {
jsi::Runtime &rt = *runtimeHelper->uiRuntime();
auto workletValue = shareableWorklet->getJSValue(rt);
runtimeHelper->runOnUIGuarded(workletValue);
});
maybeRequestRender();
}

jsi::Value NativeReanimatedModule::makeSynchronizedDataHolder(
Expand Down

0 comments on commit f8f7b2e

Please sign in to comment.