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 <john.stultz@linaro.org>
Change-Id: Iff2ca1c0b6701abbdc10c6ee92edb21a4b84a841
diff --git a/DrmHwcTwo.cpp b/DrmHwcTwo.cpp
index 413d9c3..d141f16 100644
--- a/DrmHwcTwo.cpp
+++ b/DrmHwcTwo.cpp
@@ -776,8 +776,7 @@
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 @@
supported(__func__);
set_buffer(buffer);
- acquire_fence_ = UniqueFd(fcntl(acquire_fence, F_DUPFD_CLOEXEC));
+ acquire_fence_ = UniqueFd(acquire_fence);
return HWC2::Error::None;
}