From 8adf544306db75a3fb0843b9977693d2a817bf5d Mon Sep 17 00:00:00 2001 From: John Stultz Date: Sat, 31 Jul 2021 00:19:50 +0000 Subject: [PATCH] drm_hwcomposer: Fix sync_file fd leak from "Rework audofd" In commit 0fade37afd60 ("drm_hwcomposer: Rework autofd"), a change to some very subtle existing code caused a resource leak such that after 10 minutes or so of active display output, we would start seeing: android.hardware.graphics.composer@2.3-service: failed to dup fence 10 over and over in logcat. Moving the mouse would cause black frame to flicker and eventually Surfaceflinger would crash and restart. The problem was subtle change in the following hunk: @@ -1047,10 +1049,9 @@ HWC2::Error DrmHwcTwo::HwcLayer::SetLayerBlendMode(int32_t mode) { HWC2::Error DrmHwcTwo::HwcLayer::SetLayerBuffer(buffer_handle_t buffer, int32_t acquire_fence) { supported(__func__); - UniqueFd uf(acquire_fence); set_buffer(buffer); - set_acquire_fence(uf.get()); + acquire_fence_ = UniqueFd(fcntl(acquire_fence, F_DUPFD_CLOEXEC)); return HWC2::Error::None; } The core of the problem being, that the UniqueFd class calls close(fd_) in its descructor. So while the change using fcntl(...,F_DUPFD_CLOEXEC) matches what was burried in set_acquire_fence(), dropping the creation (and more importantly the destruction when it goes out of scope) of uf changes the logic so we don't end up calling close on the aquire_fence fd argument passed in. One can confirm this resource leak by doing: adb shell lsof | grep composer And noticing the number of sync_file fds growing over time. Thus, this patch fixes the logic, so instead of dup()'ing the passed in fd, (and then closing it as done before Roman's patch), we can just set aquire_fence_ to a new UniqueFd directly using the aquire_fence fd passed in. This pattern actually occured twice, so I've fixed it in both places. Fixes: 0fade37afd60 ("drm_hwcomposer: Rework autofd") Signed-off-by: John Stultz Change-Id: Iff2ca1c0b6701abbdc10c6ee92edb21a4b84a841 --- DrmHwcTwo.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/DrmHwcTwo.cpp b/DrmHwcTwo.cpp index 413d9c3..d141f16 100644 --- a/DrmHwcTwo.cpp +++ b/DrmHwcTwo.cpp @@ -776,8 +776,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetClientTarget(buffer_handle_t target, supported(__func__); client_layer_.set_buffer(target); - client_layer_.acquire_fence_ = UniqueFd( - fcntl(acquire_fence, F_DUPFD_CLOEXEC)); + client_layer_.acquire_fence_ = UniqueFd(acquire_fence); client_layer_.SetLayerDataspace(dataspace); /* TODO: Do not update source_crop every call. @@ -1063,7 +1062,7 @@ HWC2::Error DrmHwcTwo::HwcLayer::SetLayerBuffer(buffer_handle_t buffer, supported(__func__); set_buffer(buffer); - acquire_fence_ = UniqueFd(fcntl(acquire_fence, F_DUPFD_CLOEXEC)); + acquire_fence_ = UniqueFd(acquire_fence); return HWC2::Error::None; } -- 2.11.0