games-on-whales/gst-wayland-display

Fork 0

mirror of https://github.com/games-on-whales/gst-wayland-display.git synced 2026-10-15 12:51:57 +00:00

[PR #60] fix(compositor): close the seat keymap memfd at session teardown #61

New issue

Open

opened 2026-10-15 12:48:02 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:48:02 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/games-on-whales/gst-wayland-display/pull/60
Author: @salty2011
Created: 8/19/2026
Status: 🔄 Open

Base: stable ← Head: fix/keymap-memfd-teardown

📝 Commits (1)

6ab6dbc fix(compositor): close the seat keymap memfd at session teardown

📊 Changes

3 files changed (+205 additions, -1 deletions)

View changed files

📝 wayland-display-core/src/comp/mod.rs (+42 -1)
📝 wayland-display-core/src/tests/mod.rs (+1 -0)
➕ wayland-display-core/src/tests/test_keymap_fd.rs (+162 -0)

📄 Description

Problem

smithay mints one sealed memfd:smithay-keymap per Seat::add_keyboard and keeps it in the Arc<KbdRc> behind the seat's KeyboardHandle. Dropping State closes it only when no grab is left installed.

The cycle:

The keyboard's own grab slot lives inside that same KbdRc (input/keyboard/mod.rs, KbdInternal::grab).
Every grab smithay hands out carries a clone of the handle it is installed on. PopupKeyboardGrab wraps a PopupGrab whose keyboard_handle is exactly that clone.
We install one for every xdg_popup.grab (wayland/handlers/xdg.rs).

So a grab still active when the session ends is an Arc pointing at itself, and the fd stays open for the life of the process. The escape hatches do not save it: the stale-grab discard in with_grab and PopupKeyboardGrab's has_ended() unset both run on the next grab access, which never comes once the session is over.

Measured on a workload whose clients routinely end a session with a popup grab open: 136 back-to-back sessions, exactly one leaked fd each, slope 1.00/cycle with no plateau. Everything else the compositor owns was released on schedule, including the sibling smithay-dmabuffeedback-format-table memfd in the same Display, which is what rules out a leaked State or Display. At ~50 sessions a day a host crosses the 1024 fd soft limit in under a month and new sessions fail with EMFILE.

Fix

State::release_seat, called once the event loop stops: unset any surviving grab (this is what breaks the cycle), clear keyboard focus, drop the seat's handle. The fd closes deterministically rather than depending on the client having torn its popups down politely.

Tests

Count the process's open smithay-keymap memfds across 8 teardowns, once plainly and once with a grab of PopupGrab's retaining shape installed. Neutering release_seat fails the grab test and leaves the plain one passing, which is the expected split since the plain case has no cycle to leak.

The count is process-global while cargo runs this binary's tests in parallel, so the post-cycle sample is the lowest of several readings. A leak is permanent and survives every sample; a neighbouring test's fixture is not. Without that, the two tests contaminate each other's baseline.

Known gap

Neither test goes through comp::init, so the production call site is wired but not covered. Deleting the call would leave the suite green.

Upstream candidate

The deeper fix belongs in smithay: hold the grab's KeyboardHandle weakly, or unset the grab when the last non-grab reference goes away. This is the compositor-side workaround.

Verified

cargo fmt clean, cargo check clean, 25 tests pass.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/games-on-whales/gst-wayland-display/pull/60 **Author:** [@salty2011](https://github.com/salty2011) **Created:** 8/19/2026 **Status:** 🔄 Open **Base:** `stable` ← **Head:** `fix/keymap-memfd-teardown` --- ### 📝 Commits (1) - [`6ab6dbc`](https://github.com/games-on-whales/gst-wayland-display/commit/6ab6dbc847d073156a30eb0802496e832942b3f9) fix(compositor): close the seat keymap memfd at session teardown ### 📊 Changes **3 files changed** (+205 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `wayland-display-core/src/comp/mod.rs` (+42 -1) 📝 `wayland-display-core/src/tests/mod.rs` (+1 -0) ➕ `wayland-display-core/src/tests/test_keymap_fd.rs` (+162 -0) </details> ### 📄 Description ## Problem smithay mints one sealed `memfd:smithay-keymap` per `Seat::add_keyboard` and keeps it in the `Arc<KbdRc>` behind the seat's `KeyboardHandle`. Dropping `State` closes it only when no grab is left installed. The cycle: - The keyboard's own grab slot lives inside that same `KbdRc` (`input/keyboard/mod.rs`, `KbdInternal::grab`). - Every grab smithay hands out carries a clone of the handle it is installed on. `PopupKeyboardGrab` wraps a `PopupGrab` whose `keyboard_handle` is exactly that clone. - We install one for every `xdg_popup.grab` (`wayland/handlers/xdg.rs`). So a grab still active when the session ends is an `Arc` pointing at itself, and the fd stays open for the life of the process. The escape hatches do not save it: the stale-grab discard in `with_grab` and `PopupKeyboardGrab`'s `has_ended()` unset both run on the *next* grab access, which never comes once the session is over. Measured on a workload whose clients routinely end a session with a popup grab open: 136 back-to-back sessions, exactly one leaked fd each, slope 1.00/cycle with no plateau. Everything else the compositor owns was released on schedule, including the sibling `smithay-dmabuffeedback-format-table` memfd in the same `Display`, which is what rules out a leaked `State` or `Display`. At ~50 sessions a day a host crosses the 1024 fd soft limit in under a month and new sessions fail with `EMFILE`. ## Fix `State::release_seat`, called once the event loop stops: unset any surviving grab (this is what breaks the cycle), clear keyboard focus, drop the seat's handle. The fd closes deterministically rather than depending on the client having torn its popups down politely. ## Tests Count the process's open `smithay-keymap` memfds across 8 teardowns, once plainly and once with a grab of `PopupGrab`'s retaining shape installed. Neutering `release_seat` fails the grab test and leaves the plain one passing, which is the expected split since the plain case has no cycle to leak. The count is process-global while cargo runs this binary's tests in parallel, so the post-cycle sample is the lowest of several readings. A leak is permanent and survives every sample; a neighbouring test's fixture is not. Without that, the two tests contaminate each other's baseline. ## Known gap Neither test goes through `comp::init`, so the production call site is wired but not covered. Deleting the call would leave the suite green. ## Upstream candidate The deeper fix belongs in smithay: hold the grab's `KeyboardHandle` weakly, or unset the grab when the last non-grab reference goes away. This is the compositor-side workaround. ## Verified `cargo fmt` clean, `cargo check` clean, 25 tests pass. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-15 12:48:02 +00:00

No labels

enhancement

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

games-on-whales/gst-wayland-display#61

No description provided.

Rows
Columns