SF: Clean up HWC2::Layer ownership
Changes the HWC2::Display::createLayer() call to return a shared_ptr
instead of a bare pointer. Also if the HWC2::Display is disconnected,
instead of directly destroying the HWC2::Layers associated with the
display, a new call is made to clear the display information from the
layer. For safety, checks are added to take an early out if the display
information isn't set for any call where it is used.
The CompositionEngine code was already creating a shared_ptr for the
createLayer call, and expecting the pointer to be valid until it
released it via destroyLayer.
However a hotplug disconnect could leave the CompositionEngine code
holding an invalid pointer until the next display refresh, and it could
lead to bad memory accesses if the pointer was dereferenced.
CompositionEngine itself did not do so -- it released its ownership of
the layer (and the associated display) without accessing them. This
seems to have only affected "dumpsys SurfaceFlinger", if it happened to
be executed after the disconnect, and before another refresh, and only
because it tried to print out the HWC layer id.
Bug: 181061001
Test: libsurfaceflinger_unittest
Test: libcompositionengine_test
Test: dumpsys SurfaceFlinger # after hotplug event
Change-Id: I508d6aa8ef7a6af848dd54198408d5f311175070
Merged-In: I508d6aa8ef7a6af848dd54198408d5f311175070
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index 8c8331c..953eb76 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -192,16 +192,7 @@
if (const auto halDisplayId = HalDisplayId::tryCast(mId);
outputLayer && !mIsDisconnected && halDisplayId) {
auto& hwc = getCompositionEngine().getHwComposer();
- // Note: For the moment we ensure it is safe to take a reference to the
- // HWComposer implementation by destroying all the OutputLayers (and
- // hence the HWC2::Layers they own) before setting a new HWComposer. See
- // for example SurfaceFlinger::updateVrFlinger().
- // TODO(b/121291683): Make this safer.
- auto hwcLayer =
- std::shared_ptr<HWC2::Layer>(hwc.createLayer(*halDisplayId),
- [&hwc, id = *halDisplayId](HWC2::Layer* layer) {
- hwc.destroyLayer(id, layer);
- });
+ auto hwcLayer = hwc.createLayer(*halDisplayId);
ALOGE_IF(!hwcLayer, "Failed to create a HWC layer for a HWC supported display %s",
getName().c_str());
outputLayer->setHwcLayer(std::move(hwcLayer));