commit | 8adf544306db75a3fb0843b9977693d2a817bf5d | [log] [tgz] |
---|---|---|
author | John Stultz <john.stultz@linaro.org> | Sat Jul 31 00:19:50 2021 +0000 |
committer | John Stultz <john.stultz@linaro.org> | Sat Jul 31 04:04:28 2021 +0000 |
tree | b95f6edbc059a965fc4a2d94058d3282156b0008 | |
parent | 3e2f9e725a90d7fd5abd13693e3b450b8afb3f5b [diff] |
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
Patches to drm_hwcomposer are very much welcome, we really want this to be the universal HW composer implementation for Android and similar platforms. So please bring on porting patches, bugfixes, improvements for documentation and new features.
A short list of contribution guidelines:
Submit changes via gitlab merge requests on gitlab.freedesktop.org.
drm_hwcomposer is Apache 2.0 Licensed and we require contributions to follow the developer's certificate of origin: http://developercertificate.org/.
When submitting new code please follow the naming conventions documented in the generated documentation. Also please make full use of all the helpers and convenience macros provided by drm_hwcomposer. The below command can help you with formatting of your patches:
git diff | clang-format-diff-11 -p 1 -style=file
Hardware specific changes should be tested on relevant platforms before committing.
If you need inspiration, please checkout our TODO issues.
Happy hacking!