Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -133,4 +133,8 @@ void RuntimeScheduler::setIntersectionObserverDelegate(
intersectionObserverDelegate);
}

void RuntimeScheduler::clear() noexcept {
return runtimeSchedulerImpl_->clear();
}

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class RuntimeSchedulerBase {
virtual void setIntersectionObserverDelegate(
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) = 0;
virtual void clear() noexcept = 0;
};

// This is a proxy for RuntimeScheduler implementation, which will be selected
Expand Down Expand Up @@ -180,6 +181,8 @@ class RuntimeScheduler final : RuntimeSchedulerBase {
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) override;

void clear() noexcept override;

private:
// Actual implementation, stored as a unique pointer to simplify memory
// management.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,12 @@ void RuntimeScheduler_Legacy::setIntersectionObserverDelegate(
// No-op in the legacy scheduler
}

void RuntimeScheduler_Legacy::clear() noexcept {
// No-op: the legacy scheduler runs rendering updates synchronously in
// `scheduleRenderingUpdate`, so there is no queue to drain. See the header
// for details.
}

#pragma mark - Private

void RuntimeScheduler_Legacy::scheduleWorkLoopIfNecessary() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase {
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) override;

void clear() noexcept override;

private:
std::priority_queue<
std::shared_ptr<Task>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include "RuntimeScheduler_Modern.h"

#include <glog/logging.h>

#include <ReactCommon/RuntimeExecutorSyncUIThreadUtils.h>
#include <cxxreact/TraceSection.h>
#include <jsinspector-modern/tracing/EventLoopReporter.h>
Expand Down Expand Up @@ -189,6 +191,7 @@ void RuntimeScheduler_Modern::scheduleRenderingUpdate(
RuntimeSchedulerRenderingUpdate&& renderingUpdate) {
TraceSection s("RuntimeScheduler::scheduleRenderingUpdate");

std::lock_guard lock(renderingUpdatesMutex_);
surfaceIdsWithPendingRenderingUpdates_.insert(surfaceId);
pendingRenderingUpdates_.push(renderingUpdate);
}
Expand All @@ -215,6 +218,23 @@ void RuntimeScheduler_Modern::setIntersectionObserverDelegate(
intersectionObserverDelegate_ = intersectionObserverDelegate;
}

void RuntimeScheduler_Modern::clear() noexcept {
TraceSection s("RuntimeScheduler::clear");

// Drop any pending rendering updates. The callbacks captured here may
// reference state owned by the caller (e.g. the `Scheduler`'s delegate);
// dropping them here guarantees they cannot be invoked on the JS thread
// after that state is destroyed.
std::lock_guard lock(renderingUpdatesMutex_);
auto droppedUpdates = pendingRenderingUpdates_.size();
auto droppedSurfaces = surfaceIdsWithPendingRenderingUpdates_.size();
pendingRenderingUpdates_ = {};
surfaceIdsWithPendingRenderingUpdates_.clear();
LOG(WARNING) << "RuntimeScheduler_Modern::clear() dropped "
<< droppedUpdates << " pending rendering update(s) across "
<< droppedSurfaces << " surface(s).";
}

#pragma mark - Private

void RuntimeScheduler_Modern::scheduleTask(std::shared_ptr<Task> task) {
Expand Down Expand Up @@ -332,6 +352,13 @@ void RuntimeScheduler_Modern::runEventLoopTick(
void RuntimeScheduler_Modern::updateRendering() {
TraceSection s("RuntimeScheduler::updateRendering");

// Hold the lock across the entire step so that `clear()` (called from
// another thread during `Scheduler` destruction) blocks until we are done
// running the callbacks. Otherwise `clear()` could return while we are
// still about to invoke lambdas that capture raw pointers into the
// now-destroyed `Scheduler`'s delegate.
std::lock_guard lock(renderingUpdatesMutex_);

// This is the integration of the Event Timing API in the Event Loop.
// See https://w3c.github.io/event-timing/#sec-modifications-HTML
const auto eventTimingDelegate = eventTimingDelegate_.load();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <react/renderer/runtimescheduler/Task.h>
#include <atomic>
#include <memory>
#include <mutex>
#include <queue>
#include <shared_mutex>

Expand Down Expand Up @@ -155,6 +156,8 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) override;

void clear() noexcept override;

private:
std::atomic<uint_fast8_t> syncTaskRequests_{0};

Expand Down Expand Up @@ -220,6 +223,14 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
*/
bool isEventLoopScheduled_{false};

/**
* Protects `pendingRenderingUpdates_` and
* `surfaceIdsWithPendingRenderingUpdates_` so that the queue can be safely
* cleared from another thread (e.g. when the owning `Scheduler` is
* destroyed) without racing with the JS thread that normally pushes and
* drains it.
*/
std::mutex renderingUpdatesMutex_;
std::queue<RuntimeSchedulerRenderingUpdate> pendingRenderingUpdates_;
std::unordered_set<SurfaceId> surfaceIdsWithPendingRenderingUpdates_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,23 @@ Scheduler::~Scheduler() {
uiManager_->setDelegate(nullptr);
uiManager_->setAnimationDelegate(nullptr);

// After detaching from UIManager, no more calls can be made into this
// Scheduler, so nothing new will be pushed into the RuntimeScheduler's
// rendering-update queue. Drop whatever is still queued: those lambdas
// capture raw pointers to this Scheduler's delegate (via
// `uiManagerDidDispatchCommand` / `uiManagerDidFinishTransaction`), and
// would otherwise fire on the JS thread after the delegate is destroyed.
// `clear()` blocks until any in-flight `updateRendering` finishes, so it
// is safe to destroy the delegate after this point.
#if __APPLE__
if (runtimeScheduler) {
LOG(WARNING)
<< "Scheduler::~Scheduler() clearing RuntimeScheduler rendering-update queue (address: "
<< this << ").";
runtimeScheduler->clear();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe move it higher up to where the first if (runtimeScheduler) { is ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed offline

}
#endif

// Then, let's verify that the requirement was satisfied.
auto surfaceIds = std::vector<SurfaceId>{};
uiManager_->getShadowTreeRegistry().enumerate(
Expand Down
5 changes: 5 additions & 0 deletions private/cxx-public-api/ReactNativeCPP.api
Original file line number Diff line number Diff line change
Expand Up @@ -35928,6 +35928,7 @@ class RuntimeSchedulerBase {
virtual void setIntersectionObserverDelegate(
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) = 0;
virtual void clear() noexcept = 0;
};
class RuntimeScheduler final : RuntimeSchedulerBase {
public:
Expand Down Expand Up @@ -35972,6 +35973,7 @@ class RuntimeScheduler final : RuntimeSchedulerBase {
void setIntersectionObserverDelegate(
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) override;
void clear() noexcept override;
};
} // namespace facebook::react

Expand Down Expand Up @@ -36072,6 +36074,7 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase {
void setIntersectionObserverDelegate(
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) override;
void clear() noexcept override;
};
std::atomic<uint_fast8_t> runtimeAccessRequests_{0};
std::atomic_bool isSynchronous_{false};
Expand Down Expand Up @@ -36136,6 +36139,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
void setIntersectionObserverDelegate(
RuntimeSchedulerIntersectionObserverDelegate*
intersectionObserverDelegate) override;
void clear() noexcept override;
};
std::priority_queue<
std::shared_ptr<Task>,
Expand Down Expand Up @@ -36172,6 +36176,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {
HighResTimeStamp endTime);
std::function<HighResTimeStamp()> now_;
bool isEventLoopScheduled_{false};
std::mutex renderingUpdatesMutex_;
std::queue<RuntimeSchedulerRenderingUpdate> pendingRenderingUpdates_;
std::unordered_set<SurfaceId> surfaceIdsWithPendingRenderingUpdates_;
std::atomic<ShadowTreeRevisionConsistencyManager*>
Expand Down
Loading