Use atomic pointer for animationDelegate_ to prevent race during teardown

Summary:
Changelog: [internal]

[This assumption](https://fburl.com/diffusion/yeqtvxru) in `~Scheduler` might not be completely correct. There can be a race between JS thread and main thread. With main thread settings animationDelegate_ to nullptr and JS thread reading the value. Even though they always happen one after the other, JS thread can have the value of animationDelegate_ in its cache line and changing it from main thread might not invalidate it right away.

Reviewed By: JoshuaGross

Differential Revision: D29237290

fbshipit-source-id: 6cb898caf7c225cf8162e7560921b563dec514b1
This commit is contained in:
Samuel Susla
2021-06-18 14:51:29 -07:00
committed by Facebook GitHub Bot
parent d1e75517d1
commit 52a9fed3df
2 changed files with 9 additions and 7 deletions
@@ -345,8 +345,9 @@ void UIManager::configureNextLayoutAnimation(
RawValue const &config,
jsi::Value const &successCallback,
jsi::Value const &failureCallback) const {
if (animationDelegate_) {
animationDelegate_->uiManagerDidConfigureNextLayoutAnimation(
auto animationDelegate = animationDelegate_.load();
if (animationDelegate) {
animationDelegate->uiManagerDidConfigureNextLayoutAnimation(
runtime,
config,
std::move(successCallback),
@@ -435,14 +436,15 @@ void UIManager::setAnimationDelegate(UIManagerAnimationDelegate *delegate) {
}
void UIManager::stopSurfaceForAnimationDelegate(SurfaceId surfaceId) const {
if (animationDelegate_ != nullptr) {
animationDelegate_->stopSurface(surfaceId);
auto animationDelegate = animationDelegate_.load();
if (animationDelegate) {
animationDelegate->stopSurface(surfaceId);
}
}
void UIManager::animationTick() {
if (animationDelegate_ != nullptr &&
animationDelegate_->shouldAnimateFrame()) {
auto animationDelegate = animationDelegate_.load();
if (animationDelegate && animationDelegate->shouldAnimateFrame()) {
shadowTreeRegistry_.enumerate(
[&](ShadowTree const &shadowTree, bool &stop) {
shadowTree.notifyDelegatesOfUpdates();
@@ -184,8 +184,8 @@ class UIManager final : public ShadowTreeDelegate {
ShadowTreeRegistry const &getShadowTreeRegistry() const;
SharedComponentDescriptorRegistry componentDescriptorRegistry_;
UIManagerAnimationDelegate *animationDelegate_{nullptr};
std::atomic<UIManagerDelegate *> delegate_;
std::atomic<UIManagerAnimationDelegate *> animationDelegate_{nullptr};
RuntimeExecutor const runtimeExecutor_{};
ShadowTreeRegistry shadowTreeRegistry_{};
BackgroundExecutor const backgroundExecutor_{};