Loosen requirement that getVsyncSchedule returns an object
In some cases, a display may have been removed in between referring to
a display and when we attempt to retrieve its VsyncSchedule. Remove the
assert that there must be an associated VsyncSchedule, replacing it with
handling that is specific to the call site.
Anywhere std::null_opt is passed to getVsyncSchedule (the default),
leave the call sites unchanged. This will pick the pacesetter, which
should have a VsyncSchedule every time the method is called. This should
always return a valid VsyncSchedule. Otherwise, the old code would
LOG_ALWAYS_FATAL, while the new code would have a null pointer
dereference.
LOG_ALWAYS_FATAL if there is no associated VsyncSchedule in the
following methods. They are all called on the main thread, when the
schedule should always be available when requested. This matches the
old behavior. Where these are not documented as main-thread only, add
this annotation, since that's what makes this safe to assume.
- enableHardwareVsync
- disableHardwareVsync
- setDisplayPowerMode
- addPresentFence
In addResyncSample, which is called from a binder thread, the display
may have been removed since the callback was initiated. No-op with a
warning in this case.
In getDisplayStats, which also may be called from a binder thread, fall
back to mActiveDisplayId, just in case it is called in between Scheduler
demoting and promoting the pacesetter display. Add a test for this
method.
In the following methods, log an error and early exit. This may be more
conservative than necessary.
- resyncToHardwareVsyncLocked
- setRenderRate
In setVsyncEnabled, fail to setPendingHardwareVsyncState silently,
which matches the behavior for the call to setHWCVsyncEnabled.
Bug: 266094575
Bug: 277623302
Bug: 267636813
Bug: 278806588
Bug: 275691508
Bug: 275691150
Bug: 277364366
Test: builds, boots
Test: SurfaceFlingerGetDisplayStatsTest
Change-Id: I3541909448745b04252e88f299a4283f37e755e8
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 63a0173..316ea8e 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -406,11 +406,13 @@
void Scheduler::enableHardwareVsync(PhysicalDisplayId id) {
auto schedule = getVsyncSchedule(id);
+ LOG_ALWAYS_FATAL_IF(!schedule);
schedule->enableHardwareVsync(mSchedulerCallback);
}
void Scheduler::disableHardwareVsync(PhysicalDisplayId id, bool disallow) {
auto schedule = getVsyncSchedule(id);
+ LOG_ALWAYS_FATAL_IF(!schedule);
schedule->disableHardwareVsync(mSchedulerCallback, disallow);
}
@@ -427,7 +429,10 @@
void Scheduler::resyncToHardwareVsyncLocked(PhysicalDisplayId id, bool allowToEnable,
std::optional<Fps> refreshRate) {
const auto displayOpt = mDisplays.get(id);
- LOG_ALWAYS_FATAL_IF(!displayOpt);
+ if (!displayOpt) {
+ ALOGW("%s: Invalid display %s!", __func__, to_string(id).c_str());
+ return;
+ }
const Display& display = *displayOpt;
if (display.schedulePtr->isHardwareVsyncAllowed(allowToEnable)) {
@@ -446,7 +451,10 @@
ftl::FakeGuard guard(kMainThreadContext);
const auto displayOpt = mDisplays.get(id);
- LOG_ALWAYS_FATAL_IF(!displayOpt);
+ if (!displayOpt) {
+ ALOGW("%s: Invalid display %s!", __func__, to_string(id).c_str());
+ return;
+ }
const Display& display = *displayOpt;
const auto mode = display.selectorPtr->getActiveMode();
@@ -478,12 +486,18 @@
const auto hwcVsyncPeriod = ftl::Optional(hwcVsyncPeriodIn).transform([](nsecs_t nanos) {
return Period::fromNs(nanos);
});
- return getVsyncSchedule(id)->addResyncSample(mSchedulerCallback, TimePoint::fromNs(timestamp),
- hwcVsyncPeriod);
+ auto schedule = getVsyncSchedule(id);
+ if (!schedule) {
+ ALOGW("%s: Invalid display %s!", __func__, to_string(id).c_str());
+ return false;
+ }
+ return schedule->addResyncSample(mSchedulerCallback, TimePoint::fromNs(timestamp),
+ hwcVsyncPeriod);
}
void Scheduler::addPresentFence(PhysicalDisplayId id, std::shared_ptr<FenceTime> fence) {
auto schedule = getVsyncSchedule(id);
+ LOG_ALWAYS_FATAL_IF(!schedule);
const bool needMoreSignals = schedule->getController().addPresentFence(std::move(fence));
if (needMoreSignals) {
schedule->enableHardwareVsync(mSchedulerCallback);
@@ -553,6 +567,7 @@
{
std::scoped_lock lock(mDisplayLock);
auto vsyncSchedule = getVsyncScheduleLocked(id);
+ LOG_ALWAYS_FATAL_IF(!vsyncSchedule);
vsyncSchedule->getController().setDisplayPowerMode(powerMode);
}
if (!isPacesetter) return;
@@ -582,7 +597,9 @@
}
const auto displayOpt = mDisplays.get(*idOpt);
- LOG_ALWAYS_FATAL_IF(!displayOpt);
+ if (!displayOpt) {
+ return nullptr;
+ }
return displayOpt->get().schedulePtr;
}