Skip to content

hwcomposer: Fix per-frame acquire fence fd leaks - #77

Open
MichaelMKenny wants to merge 2 commits into
waydroid:lineage-20from
MichaelMKenny:pr/fence-leak-fixes
Open

hwcomposer: Fix per-frame acquire fence fd leaks#77
MichaelMKenny wants to merge 2 commits into
waydroid:lineage-20from
MichaelMKenny:pr/fence-leak-fixes

Conversation

@MichaelMKenny

Copy link
Copy Markdown

Hi,
I wanted smooth momentum-based scrolling in Waydroid on trackpads, and thought I'd talk to Claude Fable 5 about the feasibility of this, and one thing led to another, and later that night I had fixes for Waydroid dying after minutes of use with the new smooth scrolling code. This PR is Claude's work on the fd leak. The leak is more noticable when scrolling/flinging at high momentum, especially at 120Hz. This is the PR that Claude suggested creating first, as it compliments the actual 1:1 touch-based smooth scrolling PR that goes with it.

I was hesitant of actually creating these PRs, as I'm on the autistic spectrum (and have severe anxiety) and mostly keep to myself, rarely making PRs. Especially such complicated PRs made with AI. I've made a couple or so AI PRs, but they were much smaller, and I understood them far better. I am actually a developer, but my background is iOS and Mac app development. I recently switched to Android and Linux, as I wasn't happy with how Apple was treating developers, and how closed their app ecosystem was. AI has helped me get up to speed with user Android app development, but this Android system hardware level code is kinda beyond my pay grade. My Dad is a C++ developer, but not with Android background. He has read these PRs' code, but he doesn't know this codebase. I'm only posting these PR's because it'd be a shame for these real leak fixes, and the scrolling feature to not at least have a chance of making it into Waydroid's code.

Claude did make a crash diagnostics PR that goes with these 2. I'm going to post it, but don't feel the need to use it. It raises the fd-limit, I'm not sure whether this would be to your taste. Plus, it puts a log file in the data directory, again not sure how you feel about this.

Obviously this code is riddled with AI comments, it looks like most of your code has far fewer comments in it that Claude has written. I left in as I thought it make reviewing a tad easier.

Everything below this line and the other two PRs (crash diagnostics PR, and the main smooth scrolling PR) are from Claude.

Summary

The composer process leaks one sync_file fd per rendered frame in the
compositing window modes whenever SurfaceFlinger sends skipped layers (which
it does on every frame of normal app use). After a few minutes of continuous
rendering the process hits RLIMIT_NOFILE, the Wayland connection dies with
EMFILE, hwc_wayland_thread aborts by design, and the whole session goes
down. Measured on a 120Hz host: ~28,000 leaked fds in an 8-minute session;
with these fixes the process holds steady at ~108 fds under the same load.

Root cause

The framebuffer target is drawn — and its acquire fence closed — at the
first skipped layer's position in handle_layer. But hwc_set only routed
layers with a buffer handle to the mode, and skipped layers usually carry
none, so the draw never ran; the framebuffer target's own branch then
skipped its close on the grounds that the draw site had done it. Since
HWC2On1Adapter hands the HAL a fresh dup() of the client target fence on
every set(), one fd leaked per frame. A second symptom of the same bug:
the client-composited content (everything SurfaceFlinger rendered into the
target for the skipped range) was silently never displayed.

Changes

  1. hwcomposer: Close the cursor layer's acquire fence when not rendering it
    — the non-subsurface cursor handler drops the fence entirely when no
    wl_pointer is bound (regression from the cursor handler rework; the
    subsurface handler kept the matching close).
  2. hwcomposer: Fix framebuffer target fence leak, sweep unclosed fences
    — route every non-cursor layer through the mode (all modes already
    tolerate handle-less layers and close their fences;
    multi_window_mode::can_handle_layer now requires a handle explicitly),
    close the target's fence in the non-compositing modes on frames where it
    is not drawn, and add an end-of-hwc_set sweep that closes and reports
    (rate-limited) any fence every other path missed, so a future regression
    degrades to a log line instead of a session crash. All close sites now
    reset acquireFenceFd to -1, which is what makes the sweep sound.

Behavior change to be aware of

Because the framebuffer target now actually renders when skipped layers are
present, client-composited content that was previously (incorrectly)
invisible now shows: most noticeably Android's app-launch dim scrim briefly
appears behind freeform windows during launch animations. This is faithful
to what SurfaceFlinger requests; noting it since users may notice the
difference.

Testing

  • Verified on lineage-20 x86_64 under KWin (Plasma, 120Hz laptop): fd count
    of the composer process flat at ~108 during minutes of continuous
    scrolling/fling rendering vs. ~28,000-and-climbing before; sweep log
    count is zero.
  • The leak and the fix were confirmed with in-HAL instrumentation logging
    the layer type/flags/fence at every FBT-relevant branch (removed again in
    the final commits).

wl_cursor_cursor_handler::apply_cursor does nothing at all when no
wl_pointer is bound, dropping ownership of the cursor layer's acquire
fence: one leaked fd per frame that carries a cursor layer while no
pointer exists. Before the cursor handler rework the fence was closed
unconditionally on this path, and the subsurface cursor handler still
has the matching close.

Close the fence when the cursor is not rendered.
In compositing modes with skipped layers present - which SurfaceFlinger
produces on every frame of normal app use - the framebuffer target is
drawn, and its acquire fence closed, at the first skipped layer's
position in handle_layer. But hwc_set only routed layers with a buffer
handle to the mode, and skipped layers usually carry none, so the draw
never ran; the framebuffer target's own branch then skipped its close
on the grounds that the draw site had done it. Since HWC2On1Adapter
hands the HAL a fresh dup of the client target fence on every set(),
one fd leaked per frame - several thousand over a few minutes of
continuous scrolling - until the process hit RLIMIT_NOFILE and the
Wayland connection died with EMFILE, taking the session down. As a
second symptom, the client-composited content itself (everything
SurfaceFlinger rendered into the target for the skipped range) was
silently never displayed.

Route every non-cursor layer through the mode: all modes already
tolerate handle-less layers and close their fences, and multi-window's
can_handle_layer now requires a handle explicitly. The non-compositing
modes also close the target's fence on frames where it is not drawn.

To keep any future miss from exhausting the fd table again, every
close site now resets acquireFenceFd to -1 and hwc_set ends with a
sweep that closes and reports (rate-limited) any fence still open.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant