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
1 file changed
tree: b95f6edbc059a965fc4a2d94058d3282156b0008
  1. .ci/
  2. backend/
  3. bufferinfo/
  4. compositor/
  5. drm/
  6. include/
  7. tests/
  8. utils/
  9. .clang-format
  10. .clang-tidy
  11. .gitlab-ci.yml
  12. Android.bp
  13. DrmHwcTwo.cpp
  14. DrmHwcTwo.h
  15. MODULE_LICENSE_APACHE2
  16. NOTICE
  17. presubmit.sh
  18. README.md
README.md

drm_hwcomposer

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!