HWC2: Fix virtual displays

Close the output buffer release fence per the contract in
hwcomposer2.h.

Also changes the HWC2 device to hold a weak pointer to the display,
which allows it to properly remove it when it gets torn down.

Bug: 29945684
Change-Id: Ib1b5d81e42a63d0e65b189f7c6c62e661e0ab6ae

diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index ed8cc08..ae6ba98 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -149,7 +149,12 @@ Device::~Device()
     }
 
     for (auto element : mDisplays) {
-        auto display = element.second;
+        auto display = element.second.lock();
+        if (!display) {
+            ALOGE("~Device: Found a display (%" PRId64 " that has already been"
+                    " destroyed", element.first);
+            continue;
+        }
 
         DisplayType displayType = HWC2::DisplayType::Invalid;
         auto error = display->getType(&displayType);
@@ -208,6 +213,10 @@ Error Device::createVirtualDisplay(uint32_t width, uint32_t height,
     ALOGI("Created virtual display");
     *format = static_cast<android_pixel_format_t>(intFormat);
     *outDisplay = getDisplayById(displayId);
+    if (!*outDisplay) {
+        ALOGE("Failed to get display by id");
+        return Error::BadDisplay;
+    }
     (*outDisplay)->setVirtual();
     return Error::None;
 }
@@ -289,7 +298,10 @@ void Device::callVsync(std::shared_ptr<Display> display, nsecs_t timestamp)
 
 std::shared_ptr<Display> Device::getDisplayById(hwc2_display_t id) {
     if (mDisplays.count(id) != 0) {
-        return mDisplays.at(id);
+        auto strongDisplay = mDisplays[id].lock();
+        ALOGE_IF(!strongDisplay, "Display %" PRId64 " is in mDisplays but is no"
+                " longer alive", id);
+        return strongDisplay;
     }
 
     auto display = std::make_shared<Display>(*this, id);
@@ -430,6 +442,7 @@ void Device::destroyVirtualDisplay(hwc2_display_t display)
     auto error = static_cast<Error>(intError);
     ALOGE_IF(error != Error::None, "destroyVirtualDisplay(%" PRIu64 ") failed:"
             " %s (%d)", display, to_string(error).c_str(), intError);
+    mDisplays.erase(display);
 }
 
 // Display methods
@@ -810,6 +823,7 @@ Error Display::setOutputBuffer(const sp<GraphicBuffer>& buffer,
     auto handle = buffer->getNativeBuffer()->handle;
     int32_t intError = mDevice.mSetOutputBuffer(mDevice.mHwcDevice, mId, handle,
             fenceFd);
+    close(fenceFd);
     return static_cast<Error>(intError);
 }
 

diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h
index 8ab61e9..68781da 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -88,7 +88,8 @@ public:
     // Other Device methods
 
     // This will create a Display if one is not found, but it will not be marked
-    // as connected
+    // as connected. This Display may be null if the display has been torn down
+    // but has not been removed from the map yet.
     std::shared_ptr<Display> getDisplayById(hwc2_display_t id);
 
     bool hasCapability(HWC2::Capability capability) const;
@@ -181,7 +182,7 @@ private:
     HWC2_PFN_SET_LAYER_Z_ORDER mSetLayerZOrder;
 
     std::vector<Capability> mCapabilities;
-    std::unordered_map<hwc2_display_t, std::shared_ptr<Display>> mDisplays;
+    std::unordered_map<hwc2_display_t, std::weak_ptr<Display>> mDisplays;
 
     HotplugCallback mHotplug;
     std::vector<std::pair<std::shared_ptr<Display>, Connection>>