SF: Enforce thread safety for SurfaceFlinger class
Add thread annotations to display state and accessors. Add macros and
RAII helpers as escape hatches that are stricter and more meaningful
than NO_THREAD_SAFETY_ANALYSIS, and emit error on use of the latter.
Bug: 123715322
Test: Build
Change-Id: Ibada81998d70c940c7406ef292b2d487fb02189d
(cherry picked from commit 6c2c907548d66c699c5f51d21070c4508b30b669)
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 481f350..d9b40f4 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -289,12 +289,6 @@
// The CompositionEngine encapsulates all composition related interfaces and actions.
compositionengine::CompositionEngine& getCompositionEngine() const;
- // returns the default Display
- sp<const DisplayDevice> getDefaultDisplayDevice() {
- Mutex::Autolock _l(mStateLock);
- return getDefaultDisplayDeviceLocked();
- }
-
// Obtains a name from the texture pool, or, if the pool is empty, posts a
// synchronous message to the main thread to obtain one on the fly
uint32_t getNewTexture();
@@ -307,7 +301,7 @@
void setPrimaryVsyncEnabled(bool enabled);
// main thread function to enable/disable h/w composer event
- void setPrimaryVsyncEnabledInternal(bool enabled);
+ void setPrimaryVsyncEnabledInternal(bool enabled) REQUIRES(mStateLock);
void setVsyncEnabledInHWC(DisplayId displayId, hal::Vsync enabled);
// called on the main thread by MessageQueue when an internal message
@@ -722,8 +716,8 @@
status_t captureScreenCommon(RenderArea& renderArea, TraverseLayersFunction traverseLayers,
const sp<GraphicBuffer>& buffer, bool useIdentityTransform,
bool regionSampling, bool& outCapturedSecureLayers);
- const sp<DisplayDevice> getDisplayByIdOrLayerStack(uint64_t displayOrLayerStack);
- const sp<DisplayDevice> getDisplayByLayerStack(uint64_t layerStack);
+ sp<DisplayDevice> getDisplayByIdOrLayerStack(uint64_t displayOrLayerStack) REQUIRES(mStateLock);
+ sp<DisplayDevice> getDisplayByLayerStack(uint64_t layerStack) REQUIRES(mStateLock);
status_t captureScreenImplLocked(const RenderArea& renderArea,
TraverseLayersFunction traverseLayers,
const sp<GraphicBuffer>& buffer, bool useIdentityTransform,
@@ -751,29 +745,31 @@
// called when starting, or restarting after system_server death
void initializeDisplays();
- // NOTE: can only be called from the main thread or with mStateLock held
- sp<const DisplayDevice> getDisplayDeviceLocked(const wp<IBinder>& displayToken) const {
+ sp<const DisplayDevice> getDisplayDeviceLocked(const wp<IBinder>& displayToken) const
+ REQUIRES(mStateLock) {
return const_cast<SurfaceFlinger*>(this)->getDisplayDeviceLocked(displayToken);
}
- // NOTE: can only be called from the main thread or with mStateLock held
- sp<DisplayDevice> getDisplayDeviceLocked(const wp<IBinder>& displayToken) {
+ sp<DisplayDevice> getDisplayDeviceLocked(const wp<IBinder>& displayToken) REQUIRES(mStateLock) {
const auto it = mDisplays.find(displayToken);
return it == mDisplays.end() ? nullptr : it->second;
}
- sp<const DisplayDevice> getDefaultDisplayDeviceLocked() const {
+ sp<const DisplayDevice> getDefaultDisplayDeviceLocked() const REQUIRES(mStateLock) {
return const_cast<SurfaceFlinger*>(this)->getDefaultDisplayDeviceLocked();
}
- sp<DisplayDevice> getDefaultDisplayDeviceLocked() {
+ sp<DisplayDevice> getDefaultDisplayDeviceLocked() REQUIRES(mStateLock) {
if (const auto token = getInternalDisplayTokenLocked()) {
return getDisplayDeviceLocked(token);
}
return nullptr;
}
- std::optional<DeviceProductInfo> getDeviceProductInfoLocked(DisplayId) const;
+ sp<const DisplayDevice> getDefaultDisplayDevice() EXCLUDES(mStateLock) {
+ Mutex::Autolock lock(mStateLock);
+ return getDefaultDisplayDeviceLocked();
+ }
// mark a region of a layer stack dirty. this updates the dirty
// region of all screens presenting this layer stack.
@@ -827,10 +823,11 @@
std::shared_ptr<compositionengine::Display> compositionDisplay,
const DisplayDeviceState& state,
const sp<compositionengine::DisplaySurface>& displaySurface,
- const sp<IGraphicBufferProducer>& producer);
+ const sp<IGraphicBufferProducer>& producer) REQUIRES(mStateLock);
void processDisplayChangesLocked() REQUIRES(mStateLock);
- void processDisplayAdded(const wp<IBinder>& displayToken, const DisplayDeviceState& state);
- void processDisplayRemoved(const wp<IBinder>& displayToken);
+ void processDisplayAdded(const wp<IBinder>& displayToken, const DisplayDeviceState&)
+ REQUIRES(mStateLock);
+ void processDisplayRemoved(const wp<IBinder>& displayToken) REQUIRES(mStateLock);
void processDisplayChanged(const wp<IBinder>& displayToken,
const DisplayDeviceState& currentState,
const DisplayDeviceState& drawingState) REQUIRES(mStateLock);
@@ -873,12 +870,13 @@
/*
* Display identification
*/
- sp<IBinder> getPhysicalDisplayTokenLocked(DisplayId displayId) const {
+ sp<IBinder> getPhysicalDisplayTokenLocked(DisplayId displayId) const REQUIRES(mStateLock) {
const auto it = mPhysicalDisplayTokens.find(displayId);
return it != mPhysicalDisplayTokens.end() ? it->second : nullptr;
}
- std::optional<DisplayId> getPhysicalDisplayIdLocked(const sp<IBinder>& displayToken) const {
+ std::optional<DisplayId> getPhysicalDisplayIdLocked(const sp<IBinder>& displayToken) const
+ REQUIRES(mStateLock) {
for (const auto& [id, token] : mPhysicalDisplayTokens) {
if (token == displayToken) {
return id;
@@ -888,12 +886,12 @@
}
// TODO(b/74619554): Remove special cases for primary display.
- sp<IBinder> getInternalDisplayTokenLocked() const {
+ sp<IBinder> getInternalDisplayTokenLocked() const REQUIRES(mStateLock) {
const auto displayId = getInternalDisplayIdLocked();
return displayId ? getPhysicalDisplayTokenLocked(*displayId) : nullptr;
}
- std::optional<DisplayId> getInternalDisplayIdLocked() const {
+ std::optional<DisplayId> getInternalDisplayIdLocked() const REQUIRES(mStateLock) {
const auto hwcDisplayId = getHwComposer().getInternalHwcDisplayId();
return hwcDisplayId ? getHwComposer().toPhysicalDisplayId(*hwcDisplayId) : std::nullopt;
}
@@ -945,9 +943,9 @@
void recordBufferingStats(const std::string& layerName,
std::vector<OccupancyTracker::Segment>&& history);
void dumpBufferingStats(std::string& result) const;
- void dumpDisplayIdentificationData(std::string& result) const;
+ void dumpDisplayIdentificationData(std::string& result) const REQUIRES(mStateLock);
void dumpRawDisplayIdentificationData(const DumpArgs&, std::string& result) const;
- void dumpWideColorInfo(std::string& result) const;
+ void dumpWideColorInfo(std::string& result) const REQUIRES(mStateLock);
LayersProto dumpDrawingStateProto(uint32_t traceFlags) const;
void dumpOffscreenLayersProto(LayersProto& layersProto,
uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;
@@ -974,7 +972,7 @@
/* ------------------------------------------------------------------------
* VrFlinger
*/
- void resetDisplayState();
+ void resetDisplayState() REQUIRES(mStateLock);
// Check to see if we should handoff to vr flinger.
void updateVrFlinger();
@@ -1057,16 +1055,14 @@
hal::HWDisplayId hwcDisplayId;
hal::Connection connection = hal::Connection::INVALID;
};
- // protected by mStateLock
- std::vector<HotplugEvent> mPendingHotplugEvents;
+ std::vector<HotplugEvent> mPendingHotplugEvents GUARDED_BY(mStateLock);
// this may only be written from the main thread with mStateLock held
// it may be read from other threads with mStateLock held
- std::map<wp<IBinder>, sp<DisplayDevice>> mDisplays;
- std::unordered_map<DisplayId, sp<IBinder>> mPhysicalDisplayTokens;
+ std::map<wp<IBinder>, sp<DisplayDevice>> mDisplays GUARDED_BY(mStateLock);
+ std::unordered_map<DisplayId, sp<IBinder>> mPhysicalDisplayTokens GUARDED_BY(mStateLock);
- // protected by mStateLock
- std::unordered_map<BBinder*, wp<Layer>> mLayersByLocalBinderToken;
+ std::unordered_map<BBinder*, wp<Layer>> mLayersByLocalBinderToken GUARDED_BY(mStateLock);
// don't use a lock for these, we don't care
int mDebugRegion = 0;