unom/punktfunk
6
0
Fork 1
Code Issues 3 Pull Requests 1 Actions 2 Packages Projects Releases 48 Wiki Activity

fix(pf-vdisplay): one non-UTF-8 byte in a portal config destroyed the whole file — in the module written to prevent exactly that

Browse Source 
`portal_config::ensure_key` folded EVERY read failure into an empty string
(`read_to_string(path).unwrap_or_default()`). `upsert("", …)` then produced a file containing only
our block, the one-time backup was skipped because `!existing.is_empty()` was false, and the write
replaced the user's config — returning `Ok(true)`.

So a single Latin-1 character in a comment in `~/.config/hypr/xdph.conf` or
`~/.config/xdg-desktop-portal-wlr/config` destroyed the operator's entire portal configuration, with
no backup and no warning. The module doc says flat-writing these files "destroyed [everything else]
on first connect, silently and permanently" and that this module exists so it cannot happen; that
one line re-opened the door. The same shape hit a transient EIO on an NFS or overlay config dir.

Now: bytes are read with an explicit match, only `NotFound` may mean "empty", a non-UTF-8 config is
refused by name rather than replaced, the backup is taken by BYTES, and the write is atomic
(temp + `sync_all` + rename in the same directory, permissions carried over). Five new tests, all
running on macOS — `a_non_utf8_config_is_refused_not_replaced` fails against the old code.

Also in the wlr/Mutter family:

* **Mutter's `Primary` rebuilt kept physicals from scratch** — scale forced to 1.0, transform to 0,
  disabled heads re-enabled — so a rotated, 2x-scaled or deliberately-disabled monitor came back
  wrong, while the code went to real trouble to preserve refresh. Each head now carries its
  pre-connect scale and transform, and x advances by the LOGICAL width.

* Three availability probes read session env (`SWAYSOCK`, `XDG_CURRENT_DESKTOP`,
  `HYPRLAND_INSTANCE_SIGNATURE`) with no `ENV_LOCK` while `apply_session_env` `set_var`s the same
  keys from another thread — the glibc setenv/getenv race this crate's own lib.rs documents as UB.

* `wlroots::create_output` ran a statement before its `OutputGuard` existed, so a raced
  `wait_new_output` orphaned the output permanently — hyprland takes the guard first. The
  before/after name diff also ran outside any lock, so two concurrent creates could adopt each
  other's output. Both now run under a create lock, with a stray sweep on the failure path.

* `select_and_cast`'s timeout arm dropped the portal thread's `stop` flag un-set — the same leak
  Mutter was already fixed for. The guard is now built before the wait, in both copies.

* The xdpw chooser file was written per session and never removed, permanently shadowing the
  config's fallback with the name of an already-unplugged output. Its lifetime is now the handshake,
  not the session — scoped deliberately, because tying removal to the keepalive would let one
  session delete another's selection hours later.

* Hyprland's headless outputs are now named `PF-<pid>-<n>` and reconciled at startup, so a crashed
  host's leftovers are reclaimed while a live sibling host's outputs cannot be pulled out from under
  it. `set_monitor_rule` no longer discards hyprctl's rejection text and then hard-codes a
  GBM/dmabuf diagnosis it never verified.

* Both wlr backends silently dropped the `topology` policy axis: `Primary`/`Exclusive` was accepted,
  echoed by the mgmt API, applied on three backends and a no-op on two. They now say so.

Item 8.1: `swaymsg`, `hyprctl` and the portal `systemctl --user try-restart` calls are bounded
through `proc` with named budgets.
This commit is contained in:
enricobuehler
2026-08-11 09:22:32 +02:00
parent ef72d102b6
commit 6b33750edc
4 changed files with 790 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-vdisplay/src/vdisplay/linux/hyprland.rs
+239 -23
View File
@@ -5,9 +5,10 @@
//! protocols, so it shares the wlr virtual-input path with sway — but it needs its own IPC and
//! portal, so it is a **distinct backend** from [`super::wlroots`], not a branch inside it (D1):
//!
//! 1. `hyprctl output create headless PF-<n>` adds a named headless output — Hyprland supports
//! 1. `hyprctl output create headless PF-<pid>-<n>` adds a named headless output — Hyprland supports
//! **explicit names**, so no before/after diffing like sway's `HEADLESS-N` (D6). We poll
//! `hyprctl -j monitors` until the name shows up.
//! `hyprctl -j monitors` until the name shows up. The creator's pid rides in the name so a
//! crashed host's leftovers are attributable, and only those (see [`reclaim_leftovers_once`]).
//! 2. A monitor rule sets the client's exact mode. [`set_monitor_rule`] uses `hyprctl keyword
//! monitor NAME,WxH@Hz,auto,1` (the hyprlang path — the default config manager on every current
//! release, ≥0.55 included) and falls back to the Lua `hyprctl eval 'hl.monitor{…}'` only for a
@@ -69,12 +70,46 @@ fn picker_selection_line(name: &str) -> String {
format!("[SELECTION]screen:{name}\n")
}
/// Monotonic per-process counter for headless output names (`PF-1`, `PF-2`, …). Named outputs kill
/// the before/after diff race sway needs (D6).
/// Monotonic per-process counter for headless output names (`PF-<pid>-1`, `PF-<pid>-2`, …). Named
/// outputs kill the before/after diff race sway needs (D6).
static OUTPUT_SEQ: AtomicU32 = AtomicU32::new(0);
/// The name for our next headless output: `PF-<pid>-<n>`.
///
/// The pid is not decoration. `OutputGuard::drop` is the only thing that removes an output, so a
/// host that was SIGKILLed leaves its outputs in the compositor — and a bare `PF-<n>` counter starts
/// again at `PF-1` in the next process, colliding with the corpses it just inherited. Stamping the
/// creator's pid into the name makes a leftover both recognisable and *attributable*, which is what
/// lets [`reclaim_leftovers_once`] remove only the ones whose owner is gone.
fn next_output_name() -> String {
format!("PF-{}", OUTPUT_SEQ.fetch_add(1, Ordering::Relaxed) + 1)
format!(
"PF-{}-{}",
std::process::id(),
OUTPUT_SEQ.fetch_add(1, Ordering::Relaxed) + 1
)
}
/// Is `name` an output some punktfunk host created (`PF-<pid>-<n>`, or a legacy `PF-<n>`)? Pure —
/// this is what [`list_monitors`] reports as `managed`, so a user's own monitor called `PF-office`
/// must not qualify.
fn is_managed_output(name: &str) -> bool {
let Some(rest) = name.strip_prefix("PF-") else {
return false;
};
!rest.is_empty()
&& rest
.split('-')
.all(|part| !part.is_empty() && part.bytes().all(|b| b.is_ascii_digit()))
}
/// The pid of the host that created `name`, for `PF-<pid>-<n>` only. `None` for anything else —
/// including a legacy `PF-<n>` from a host older than this naming scheme, which carries no owner and
/// therefore may not be reclaimed on a guess.
fn output_owner_pid(name: &str) -> Option<u32> {
let rest = name.strip_prefix("PF-")?;
let (pid, seq) = rest.split_once('-')?;
seq.parse::<u32>().ok()?;
pid.parse::<u32>().ok()
}
/// The Hyprland virtual-display driver. Stateless — each [`create`](VirtualDisplay::create) adds one
@@ -100,11 +135,24 @@ impl HyprlandDisplay {
/// under `$XDG_RUNTIME_DIR/hypr/*/.socket.sock` (so the systemd `--user` host works without env
/// import, unlike sway's `SWAYSOCK`; the signature is then exported by `apply_session_env`). Cheap,
/// side-effect-free — safe on the enumeration path.
///
/// Both env reads take [`crate::with_env_lock`] — in ONE scope, so the pair is sampled from a single
/// consistent view. This runs on a management worker (`/host/compositors` → [`crate::available`])
/// concurrently with another connect's `apply_session_env`, which `set_var`s the signature for a
/// live Hyprland session and `remove_var`s it for anything else; a glibc `getenv` racing that
/// `setenv`/`unsetenv` is the `environ` realloc data race ENV_LOCK exists for. No caller holds the
/// lock (it is not reentrant), and the `read_dir` below deliberately runs outside it.
pub fn is_available() -> bool {
if std::env::var_os("HYPRLAND_INSTANCE_SIGNATURE").is_some() {
let (sig, runtime) = crate::with_env_lock(|| {
(
std::env::var_os("HYPRLAND_INSTANCE_SIGNATURE"),
std::env::var_os("XDG_RUNTIME_DIR"),
)
});
if sig.is_some() {
return true;
}
let dir = match std::env::var_os("XDG_RUNTIME_DIR") {
let dir = match runtime {
Some(d) => std::path::PathBuf::from(d).join("hypr"),
None => return false,
};
@@ -147,6 +195,9 @@ impl VirtualDisplay for HyprlandDisplay {
fn create(&mut self, mode: Mode) -> Result<VirtualOutput> {
// Log the permission-system caveat once per process (silent black frames otherwise).
preflight_once();
// Remove any output a PREVIOUS host left in this compositor, before we mint our first.
reclaim_leftovers_once();
warn_topology_is_extend_only();
let name = next_output_name();
hyprctl_dispatch(&["output", "create", "headless", &name]).with_context(|| {
@@ -181,7 +232,7 @@ impl VirtualDisplay for HyprlandDisplay {
remote_fd: Some(fd),
preferred_mode: Some((mode.width, mode.height, mode.refresh_hz)),
keepalive: Box::new(Keepalive {
_stop: StopGuard(stop),
_stop: stop,
_output: output,
}),
// Owned (the compositor output is ours to tear down), but not registry-poolable: the
@@ -212,6 +263,62 @@ impl Drop for StopGuard {
}
}
/// Remove the `PF-<pid>-<n>` outputs left behind by host processes that are **gone**, once per
/// process before we create our first.
///
/// [`OutputGuard::drop`] is the only unplug path there is, so a host that was SIGKILLed, OOM-killed
/// or crashed leaves its headless outputs in the compositor for as long as the Hyprland session
/// lives — a dead `PF-…` head in the operator's layout, forever, with the next host start happily
/// adding more beside it. Reclaim is keyed on the OWNER pid in the name and only removes an output
/// whose creator no longer exists, so a second live host on the same session (or this very process)
/// can never have its output pulled out from under it. `Once` puts the sweep strictly before this
/// process owns anything, and blocks a concurrent first `create` until it is done.
fn reclaim_leftovers_once() {
static RECLAIMED: Once = Once::new();
RECLAIMED.call_once(|| {
let Ok(names) = monitor_names() else { return };
for name in names {
let Some(pid) = output_owner_pid(&name) else {
// Either not ours, or a legacy `PF-<n>` with no owner recorded — which we must not
// remove on a guess, because a still-running older host may be streaming it.
if is_managed_output(&name) {
tracing::debug!(output = %name, "a managed headless output with no owner pid in \
its name (an older host build) — left alone");
}
continue;
};
if pid == std::process::id() || std::path::Path::new(&format!("/proc/{pid}")).exists() {
continue;
}
match hyprctl_dispatch(&["output", "remove", &name]) {
Ok(()) => tracing::info!(output = %name, owner_pid = pid, "removed a headless \
output left behind by a host that is no longer running"),
Err(e) => tracing::warn!(output = %name, owner_pid = pid, error = %format!("{e:#}"),
"could not remove a leftover headless output"),
}
}
});
}
/// The configured [`crate::policy::Topology`] is not implemented on this backend — say so once per
/// create instead of leaving the management API's echo as the only signal that the pin was dropped
/// (sweep 13.18). The Hyprland headless output is always an EXTENSION: nothing here promotes it to
/// primary or disables the operator's heads.
fn warn_topology_is_extend_only() {
let topology = crate::effective_topology();
if !matches!(
topology,
crate::policy::Topology::Extend | crate::policy::Topology::Auto
) {
tracing::warn!(
?topology,
"hyprland: this backend implements EXTEND only — the headless output is added beside \
the operator's heads and nothing is promoted or disabled. Configure `topology: extend` \
to stop the console promising otherwise."
);
}
}
/// Owns the created headless output; dropping it removes it from Hyprland.
struct OutputGuard(String);
@@ -226,14 +333,25 @@ impl Drop for OutputGuard {
}
}
/// Budget for one `hyprctl` call ([`crate::proc`]).
///
/// `hyprctl` is a client of the compositor it drives — it connects to the instance socket and waits
/// for a reply, so against a wedged Hyprland it never returns. These calls run on the session's
/// stream thread, whose only way to end a session is to return, so one hung query used to wedge the
/// session for good. Generous next to a healthy call (single-digit milliseconds), and every call
/// site already has a failed-query path.
const HYPRCTL_BUDGET: Duration = Duration::from_secs(5);
/// Budget for the one-shot xdph restart. `systemctl --user try-restart` waits for the user manager's
/// job to settle, so it is the slowest helper on this path — and its result is already ignored.
const PORTAL_RESTART_BUDGET: Duration = Duration::from_secs(10);
/// Run `hyprctl <args>`, returning stdout. `hyprctl` reads `HYPRLAND_INSTANCE_SIGNATURE` from the
/// env (exported by `apply_session_env`) to reach the right instance socket. It exits non-zero on a
/// hard failure, but for dispatch commands it can print an error with status 0 — see
/// [`hyprctl_dispatch`].
fn hyprctl(args: &[&str]) -> Result<String> {
let out = Command::new("hyprctl")
.args(args)
.output()
let out = crate::proc::output_within(Command::new("hyprctl").args(args), HYPRCTL_BUDGET)
.context("run hyprctl (is Hyprland installed?)")?;
if !out.status.success() {
bail!(
@@ -251,12 +369,36 @@ fn hyprctl(args: &[&str]) -> Result<String> {
/// write between ours and xdph's read would silently steer capture at the other session's output.
static SELECTION_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
/// The per-session selection file, removed when the handshake it steers is over.
///
/// Its lifetime is the HANDSHAKE, not the session: the shim cats it once, inside
/// [`select_and_cast`]'s critical section, and everything after that is the cast's own business.
/// Left behind (as it was) the stale `[SELECTION]screen:PF-…` outlives the output `Drop` has since
/// removed, and it permanently shadows xdph's documented empty-read fallback — every later capture
/// that reaches the picker without a session of ours is steered at an output that is gone. Tying
/// removal to the CAST instead would be worse: the file is one per user, so a session ending hours
/// later would delete a *sibling's* selection out from under its picker.
struct SelectionFile(String);
impl Drop for SelectionFile {
fn drop(&mut self) {
if let Err(e) = std::fs::remove_file(&self.0) {
if e.kind() != std::io::ErrorKind::NotFound {
tracing::debug!(path = %self.0, error = %e, "could not remove the xdph selection file");
}
}
}
}
/// Point xdph's custom picker at `output` and run the ScreenCast handshake, returning the portal fd
/// + node id and the guard that stops the cast. The caller must hold [`SELECTION_LOCK`].
fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, Arc<AtomicBool>)> {
fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, StopGuard)> {
ensure_xdph_config()?;
let sel = selection_file();
std::fs::write(&sel, picker_selection_line(output)).with_context(|| format!("write {sel}"))?;
// Owned from the write on: every arm below (and every `?`) leaves the handshake, which is the
// only thing that reads it.
let _sel_file = SelectionFile(sel);
let (setup_tx, setup_rx) = std::sync::mpsc::channel::<Result<(OwnedFd, u32), String>>();
let stop = Arc::new(AtomicBool::new(false));
let stop_thread = stop.clone();
@@ -264,8 +406,16 @@ fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, Arc<A
.name("punktfunk-hypr-cast".into())
.spawn(move || portal_thread(setup_tx, stop_thread, hw_cursor))
.context("spawn hyprland portal thread")?;
// Built BEFORE the wait so EVERY error arm below sets the flag on its way out — as Mutter's
// `create` does. Returning the bare `Arc` and letting the CALLER wrap it left the two failure
// arms dropping an un-set flag: the thread's `send` can still LAND in the queue in the window
// between `recv_timeout` giving up and `setup_rx` being dropped, so it reports success and then
// parks forever on `while !stop`, holding a live ScreenCast session, its zbus connection, an
// `OwnedFd` and a 2-worker tokio runtime — one more set per slow-portal connect, for the host's
// lifetime, against an output that no longer exists.
let guard = StopGuard(stop);
match setup_rx.recv_timeout(Duration::from_secs(20)) {
Ok(Ok((fd, node_id))) => Ok((fd, node_id, stop)),
Ok(Ok((fd, node_id))) => Ok((fd, node_id, guard)),
Ok(Err(e)) => bail!("ScreenCast portal on {output} failed: {e}"),
Err(_) => bail!("timed out waiting for the ScreenCast portal on {output}"),
}
@@ -285,7 +435,7 @@ pub(crate) fn stream_existing_output(
Ok(crate::mirror::MirrorStream {
node_id,
remote_fd: Some(fd),
keepalive: Box::new(StopGuard(stop)),
keepalive: Box::new(stop),
})
}
@@ -330,11 +480,12 @@ pub(crate) fn list_monitors() -> Result<Vec<crate::monitors::PhysicalMonitor>> {
.unwrap_or(1.0),
primary: m.get("focused").and_then(|v| v.as_bool()).unwrap_or(false),
enabled: !m.get("disabled").and_then(|v| v.as_bool()).unwrap_or(false),
// Our headless outputs are named `PF-<n>` (see `next_output_name`).
// Our headless outputs are named `PF-<pid>-<n>` (see `next_output_name`); the shape
// is checked, not just the prefix, so a user's own `PF-office` stays theirs.
managed: m
.get("name")
.and_then(|v| v.as_str())
.is_some_and(|n| n.starts_with("PF-")),
.is_some_and(is_managed_output),
})
})
.collect();
@@ -382,6 +533,23 @@ fn wait_monitor_ready(name: &str, timeout: Duration) -> Result<()> {
}
}
/// Every monitor name Hyprland reports, **disabled ones included** (`-j monitors all`) — a leftover
/// output from a dead host may well have ended up disabled, and [`reclaim_leftovers_once`] must see
/// it anyway.
fn monitor_names() -> Result<Vec<String>> {
let out = hyprctl(&["-j", "monitors", "all"])?;
let monitors: serde_json::Value =
serde_json::from_str(&out).context("parse hyprctl -j monitors all")?;
Ok(monitors
.as_array()
.map(|a| {
a.iter()
.filter_map(|m| m.get("name").and_then(|n| n.as_str()).map(str::to_owned))
.collect()
})
.unwrap_or_default())
}
/// Is a monitor named `name` present in `hyprctl -j monitors` (JSON)?
fn monitor_exists(name: &str) -> Result<bool> {
let out = hyprctl(&["-j", "monitors"])?;
@@ -417,17 +585,33 @@ fn set_monitor_rule(name: &str, mode: Mode) -> Result<()> {
);
let keyword: Vec<&str> = vec!["keyword", "monitor", &spec];
let eval: Vec<&str> = vec!["eval", &lua];
// What each form actually said. hyprctl reports a rejection in its OUTPUT TEXT ("eval is only
// supported with the lua config manager", "invalid monitor rule", a permission denial), and
// dropping it on the floor with `.is_err()` is what left the failure below guessing at GBM when
// the compositor had already named the real cause.
let mut attempts: Vec<String> = Vec::new();
for a in [&keyword, &eval] {
// A wrong-era command errors (`keyword` gone under Lua, or `eval` under hyprlang) — skip to
// the other form. A command that's accepted then has up to the timeout to take effect.
if hyprctl_dispatch(a).is_err() {
if let Err(e) = hyprctl_dispatch(a) {
let said = format!("{e:#}");
tracing::debug!(output = %name, cmd = ?a, error = %said, "hyprctl rejected this monitor-rule form — trying the other config era");
attempts.push(said);
continue;
}
if wait_exact_mode(name, mode, Duration::from_millis(1500)) {
tracing::debug!(output = %name, cmd = ?a, w = mode.width, h = mode.height, "monitor adopted the requested mode");
return Ok(());
}
attempts.push(format!(
"hyprctl {a:?} was accepted but the mode never took effect"
));
}
let said = if attempts.is_empty() {
"nothing (no form was attempted)".to_string()
} else {
attempts.join("; ")
};
// Neither form produced the exact mode. Distinguish "usable but different size" (proceed with a
// warning — a working stream beats none) from "0×0 / gone" (the output has no framebuffer at all).
match monitor_size(name)? {
@@ -436,14 +620,20 @@ fn set_monitor_rule(name: &str, mode: Mode) -> Result<()> {
output = %name,
requested = %format!("{}x{}", mode.width, mode.height),
got = %format!("{w}x{h}"),
hyprctl = %said,
"Hyprland did not adopt the exact requested mode — streaming at the output's current size"
);
Ok(())
}
// The output has no framebuffer at all. Lead with what hyprctl SAID: if every form was
// rejected the cause is named right there (wrong config era, a permission denial, a bad
// rule) and no allocation was ever attempted; only a form that was accepted and still left
// the output at 0×0 points at the compositor failing to back the mode.
_ => bail!(
"headless output {name} never got a framebuffer (stayed 0x0) after the monitor rule for \
{}x{}@{hz} — the compositor could not back the mode, likely a headless GBM/dmabuf \
allocation failure (GPU driver; cf. Sunshine#4197). Check the Hyprland log.",
{}x{}@{hz}. hyprctl said: {said}. If a form was accepted, the compositor could not back \
the mode — likely a headless GBM/dmabuf allocation failure (GPU driver; cf. \
Sunshine#4197). Check the Hyprland log.",
mode.width,
mode.height
),
@@ -574,13 +764,17 @@ fn ensure_xdph_config() -> Result<()> {
return Ok(());
}
tracing::info!(path = %path.display(), "pointed xdg-desktop-portal-hyprland at the managed picker shim");
let _ = Command::new("systemctl")
.args([
// Bounded: `systemctl --user` blocks on the user manager's job queue, and this runs on the
// session's stream thread. Its result was already ignored — a timeout just means xdph picks the
// new config up whenever it next starts.
let _ = crate::proc::status_within(
Command::new("systemctl").args([
"--user",
"try-restart",
"xdg-desktop-portal-hyprland.service",
])
.status();
]),
PORTAL_RESTART_BUDGET,
);
Ok(())
}
@@ -702,6 +896,28 @@ mod tests {
assert_ne!(a, b);
}
/// The name carries the creating host's pid, which is what makes a leftover attributable — a
/// reclaim that could not tell whose output it was would have to remove a LIVE sibling's or
/// nothing at all.
#[test]
fn a_name_carries_its_owner_pid_and_only_ours_does() {
let mine = next_output_name();
assert_eq!(output_owner_pid(&mine), Some(std::process::id()));
assert!(is_managed_output(&mine));
// A legacy `PF-<n>` from an older host: recognisably managed, but with no owner recorded —
// so it may be reported, never reclaimed on a guess.
assert!(is_managed_output("PF-1"));
assert_eq!(output_owner_pid("PF-1"), None);
// Not ours: a user's own monitor name that happens to start with the prefix, and the
// connectors every wlr-family compositor mints.
for theirs in ["PF-office", "PF-", "PF-12-abc", "HEADLESS-1", "DP-1", ""] {
assert!(!is_managed_output(theirs), "{theirs:?} is not ours");
assert_eq!(output_owner_pid(theirs), None, "{theirs:?} has no owner");
}
}
#[test]
fn picker_line_carries_the_selection_marker() {
// xdph requires the `[SELECTION]` prefix; a bare `screen:NAME` is rejected as strange output.
crates/pf-vdisplay/src/vdisplay/linux/mutter.rs
+160 -33
View File
@@ -122,8 +122,14 @@ impl MutterDisplay {
/// `XDG_SESSION_DESKTOP` alongside would resurrect the bug that scrub exists to prevent — a stale
/// `gnome` there after a gnome-shell crash reports Mutter usable and routes the next client into a
/// dead session (45 s create timeouts instead of a crisp handshake error).
///
/// The read takes [`crate::with_env_lock`]: this runs on a management worker (`/host/compositors` →
/// [`crate::available`]) concurrently with another connect's `apply_session_env`, which `set_var`s
/// this key for a live session and `remove_var`s it when nothing is — and a glibc `getenv` racing
/// that is the `environ` realloc data race ENV_LOCK exists for, torn answer at best and a host
/// segfault mid-connect at worst. Read-then-drop; no caller holds the lock (it is not reentrant).
pub fn is_available() -> bool {
std::env::var("XDG_CURRENT_DESKTOP")
crate::with_env_lock(|| std::env::var("XDG_CURRENT_DESKTOP"))
.map(|d| d.to_ascii_uppercase().contains("GNOME"))
.unwrap_or(false)
}
@@ -718,13 +724,24 @@ async fn connect(
}
// ---------------------------------------------------------------------------------------------
// Optional: make the per-session virtual output the PRIMARY monitor (PUNKTFUNK_MUTTER_VIRTUAL_PRIMARY).
// Optional: make the per-session virtual output the PRIMARY monitor.
//
// `RecordVirtual` adds the virtual monitor as an *extended* desktop. On a headless host that's the
// only display, so the shell + windows live there. But when a physical monitor is attached, GNOME
// keeps it primary and the virtual output is an empty extension — the stream shows only the
// wallpaper. We fix that by promoting the virtual output to primary (physical kept on, secondary)
// via `org.gnome.Mutter.DisplayConfig.ApplyMonitorsConfig`, and restore on teardown.
// wallpaper. We fix that by promoting the virtual output via
// `org.gnome.Mutter.DisplayConfig.ApplyMonitorsConfig`.
//
// Which shape is `crate::effective_topology()`'s call, not this module's: the console policy first,
// then the legacy `PUNKTFUNK_{KWIN,MUTTER}_VIRTUAL_PRIMARY` env, then the Auto default. `Primary`
// keeps the physicals on as secondaries; `Exclusive` omits them, so Mutter disables them for the
// session; `Extend` skips this block entirely.
//
// Applied at APPLY_TEMPORARY, and **MUTTER ITSELF REVERTS IT** when the virtual monitor disappears
// and our DisplayConfig connection closes. We must never re-assert the layout on teardown: the
// banner used to promise a "restore on teardown" that the teardown deliberately does not do, and
// issuing that ApplyMonitorsConfig is what SIGSEGVed gnome-shell on Mutter 50 + NVIDIA and wedged a
// box at the GDM greeter (see the teardown comment in `session_thread`).
// ---------------------------------------------------------------------------------------------
/// `org.gnome.Mutter.DisplayConfig.GetCurrentState` reply shapes (see the interface XML):
@@ -811,7 +828,9 @@ fn current_mode(state: &CurrentState, connector: &str) -> Option<(String, i32, i
/// Pure mode-pick for a KEPT physical (unit-tested). Given the physical's PRE-connect mode
/// (`pre_mode = (id, w, h, refresh)`; `None` when the connector is new since the snapshot) and the
/// mode list Mutter reports for it in the POST-virtual state
/// (`(id, w, h, refresh, is_current, is_preferred)`), return the `(mode_id, width)` to re-apply.
/// (`(id, w, h, refresh, is_current, is_preferred)`), return the `(mode_id, width, height)` to
/// re-apply. The height is not decoration: a head rotated 90°/270° is as wide on the desktop as its
/// mode is tall, and the caller lays the kept heads out side by side.
///
/// Mutter re-derives its layout when the `RecordVirtual` output appears and can silently drop a
/// 120 Hz panel to its EDID-preferred 60 Hz — so the post-virtual `is-current` is *already* 60 Hz.
@@ -821,40 +840,40 @@ fn current_mode(state: &CurrentState, connector: &str) -> Option<(String, i32, i
fn pick_keep_mode(
pre_mode: Option<(String, i32, i32, f64)>,
state_modes: &[(String, i32, i32, f64, bool, bool)],
) -> Option<(String, i32)> {
) -> Option<(String, i32, i32)> {
let state_current = || {
state_modes
.iter()
.find(|m| m.4)
.or_else(|| state_modes.iter().find(|m| m.5))
.or_else(|| state_modes.first())
.map(|m| (m.0.clone(), m.1))
.map(|m| (m.0.clone(), m.1, m.2))
};
let Some((pre_id, w, h, hz)) = pre_mode else {
return state_current();
};
// The exact pre mode id, if the connector still offers it (same session ⇒ usually true).
if state_modes.iter().any(|m| m.0 == pre_id) {
return Some((pre_id, w));
return Some((pre_id, w, h));
}
// Else a re-keyed id with the same geometry + refresh (still the real 120 Hz).
if let Some(m) = state_modes
.iter()
.find(|m| m.1 == w && m.2 == h && (m.3 - hz).abs() < 0.5)
{
return Some((m.0.clone(), m.1));
return Some((m.0.clone(), m.1, m.2));
}
// The physical genuinely no longer offers that mode — use whatever is valid now.
state_current()
}
/// The `(mode_id, width)` a kept physical should be RE-APPLIED at — its PRE-connect mode preserved
/// across Mutter's virtual-output layout re-derive. See [`pick_keep_mode`].
/// The `(mode_id, width, height)` a kept physical should be RE-APPLIED at — its PRE-connect mode
/// preserved across Mutter's virtual-output layout re-derive. See [`pick_keep_mode`].
fn physical_keep_mode(
pre: &CurrentState,
state: &CurrentState,
conn: &str,
) -> Option<(String, i32)> {
) -> Option<(String, i32, i32)> {
let pre_mode = current_mode_full(pre, conn);
let state_modes: Vec<(String, i32, i32, f64, bool, bool)> = state
.1
@@ -1044,13 +1063,57 @@ fn snap_integral_scale(want: f64, width: u32, height: u32) -> f64 {
.unwrap_or(want)
}
/// The scale of the logical monitor carrying `connector`, if present.
fn logical_scale(state: &CurrentState, connector: &str) -> Option<f64> {
/// The `(scale, transform)` of the logical monitor carrying `connector`. `None` means **no logical
/// monitor carries it** — which is how Mutter reports a head the operator has DISABLED, and is the
/// distinction [`keep_head_layout`] turns into "leave it off".
fn logical_placement(state: &CurrentState, connector: &str) -> Option<(f64, u32)> {
state
.2
.iter()
.find(|l| l.5.iter().any(|spec| spec.0 == connector))
.map(|l| l.2)
.map(|l| (l.2, l.3))
}
/// The scale of the logical monitor carrying `connector`, if present.
fn logical_scale(state: &CurrentState, connector: &str) -> Option<f64> {
logical_placement(state, connector).map(|(scale, _)| scale)
}
/// Whether a kept physical should be re-applied at all, and with what `(scale, transform)`. Pure —
/// unit-tested, because getting it wrong is invisible on a headless lab box and very visible on the
/// operator's desk.
///
/// The rebuild used to hardcode `scale = 1.0`, `transform = 0` and to list every connector Mutter
/// reported, so one connect un-rotated a portrait panel, dropped a 2×-scaled 4K head to native
/// pixels, and switched a deliberately-dark monitor back on. All three facts are in the PRE-connect
/// snapshot: `pre_logical` is the head's logical-monitor entry there, and Mutter reports a disabled
/// head by omitting it from `logical_monitors` entirely. So: carry the pre values when the head was
/// on; leave it out when the connector existed pre-connect and carried no logical monitor (disabled
/// on purpose); and for a connector that was not in the snapshot at all — a hotplug inside our
/// window — keep it on at whatever Mutter has just derived for it, which is the friendlier reading
/// of "the operator plugged this in while we were connecting".
fn keep_head_layout(
existed_pre: bool,
pre_logical: Option<(f64, u32)>,
state_logical: Option<(f64, u32)>,
) -> Option<(f64, u32)> {
// A non-finite or non-positive scale would fail the whole ApplyMonitorsConfig, taking the
// primary switch down with it.
let sane = |(scale, transform): (f64, u32)| {
(
if scale.is_finite() && scale > 0.0 {
scale
} else {
1.0
},
transform,
)
};
match (pre_logical, existed_pre) {
(Some(l), _) => Some(sane(l)),
(None, true) => None,
(None, false) => Some(sane(state_logical.unwrap_or((1.0, 0)))),
}
}
/// Every head Mutter reports, for [`crate::monitors::list`].
@@ -1142,16 +1205,20 @@ fn build_exclusive_config(vconn: &str, vmode: &str, scale: f64) -> Vec<ApplyLogi
)]
}
/// **Primary** — the virtual output primary at `(0, 0)`, with every currently-active physical
/// monitor KEPT as a secondary (laid left-to-right past the virtual, each at its **pre-connect**
/// mode). So the shell + new windows land on the streamed surface, but the operator's physical
/// screen stays on **at its real refresh**. On a headless host (no physicals) this is identical to
/// [`build_exclusive_config`].
/// **Primary** — the virtual output primary at `(0, 0)`, with every physical monitor the operator
/// had ENABLED kept as a secondary (laid left-to-right past the virtual, each at its **pre-connect**
/// mode, scale and transform). So the shell + new windows land on the streamed surface, but the
/// operator's physical screen stays exactly as they left it. On a headless host (no physicals) this
/// is identical to [`build_exclusive_config`].
///
/// `pre` is the snapshot taken *before* the virtual output existed (physical still at its true
/// refresh); `state` is the post-virtual state. We read each physical's mode from `pre` because
/// Mutter can knock a 120 Hz panel down to 60 Hz when it re-derives the layout for the virtual
/// monitor — reading `state` would cement that 60 Hz (`physical_keep_mode`).
/// refresh); `state` is the post-virtual state. Everything about a kept head is read from `pre`,
/// because the post-virtual state is already contaminated: Mutter re-derives the layout when the
/// `RecordVirtual` output appears and can knock a 120 Hz panel down to 60 Hz, so reading `state`
/// would cement that 60 Hz (`physical_keep_mode`). Scale, transform and enabled-ness come from the
/// same snapshot for the same reason — and because rebuilding them from scratch is what used to
/// un-rotate portrait panels, flatten a 2× scale and re-light a head the operator had switched off
/// ([`keep_head_layout`]).
///
/// *Physical-keep is unvalidated on-glass* — the lab boxes are headless (no attached display to keep
/// on); the layout math is conservative (append to the right) but wants a display-attached box.
@@ -1190,16 +1257,42 @@ fn build_primary_keeping_physicals(
if conn == vconn {
continue;
}
if let Some((mode_id, w)) = physical_keep_mode(pre, state, conn) {
let existed_pre = pre.1.iter().any(|m| m.0 .0 == *conn);
let Some((head_scale, transform)) = keep_head_layout(
existed_pre,
logical_placement(pre, conn),
logical_placement(state, conn),
) else {
// Omitted from the config ⇒ Mutter leaves it disabled, which is what the operator asked
// for. Listing it would switch their dark head on for the length of the session.
tracing::debug!(
connector = %conn,
"mutter: this head was disabled before the session — leaving it disabled"
);
continue;
};
if let Some((mode_id, w, h)) = physical_keep_mode(pre, state, conn) {
logicals.push((
x,
0,
1.0,
0,
head_scale,
transform,
false,
vec![(conn.clone(), mode_id, HashMap::new())],
));
x += w.max(0);
// Advance by the head's own LOGICAL footprint, in the layout's coordinate space — the
// same space the virtual's advance above uses. A 3840-wide panel at scale 2 occupies
// 1920, and a head rotated 90°/270° (transform 1/3, or their flipped twins 5/7) is as
// wide as its mode is TALL. Advancing by raw mode width was only ever *consistent* with
// the forced scale of 1.0 this rebuild used to apply; preserving the real scale without
// this would just trade one wrong layout for another (overlapping or gapped heads).
let rotated = matches!(transform, 1 | 3 | 5 | 7);
let footprint = if rotated { h } else { w };
x += if physical_layout {
footprint.max(0)
} else {
((footprint as f64 / head_scale).round() as i32).max(0)
};
}
}
logicals
@@ -1207,7 +1300,10 @@ fn build_primary_keeping_physicals(
#[cfg(test)]
mod tests {
use super::{pick_keep_mode, pick_virtual, snap_integral_scale, HashMap, Mode, MonitorInfo};
use super::{
keep_head_layout, pick_keep_mode, pick_virtual, snap_integral_scale, HashMap, Mode,
MonitorInfo,
};
// (id, w, h, refresh, is_current, is_preferred)
fn m(
@@ -1232,7 +1328,7 @@ mod tests {
];
assert_eq!(
pick_keep_mode(pre, &state),
Some(("M120".to_string(), 2560))
Some(("M120".to_string(), 2560, 1440))
);
}
@@ -1247,7 +1343,7 @@ mod tests {
];
assert_eq!(
pick_keep_mode(pre, &state),
Some(("new-120".to_string(), 2560))
Some(("new-120".to_string(), 2560, 1440))
);
}
@@ -1262,7 +1358,7 @@ mod tests {
];
assert_eq!(
pick_keep_mode(pre, &state),
Some(("s-100".to_string(), 3440))
Some(("s-100".to_string(), 3440, 1440))
);
}
@@ -1289,7 +1385,10 @@ mod tests {
m("A", 1920, 1080, 60.0, true, false),
m("B", 1920, 1080, 144.0, false, true),
];
assert_eq!(pick_keep_mode(None, &state), Some(("A".to_string(), 1920)));
assert_eq!(
pick_keep_mode(None, &state),
Some(("A".to_string(), 1920, 1080))
);
let no_current = vec![
m("A", 1920, 1080, 60.0, false, false),
@@ -1297,7 +1396,35 @@ mod tests {
];
assert_eq!(
pick_keep_mode(None, &no_current),
Some(("B".to_string(), 1920))
Some(("B".to_string(), 1920, 1080))
);
}
/// A kept physical must come back exactly as the operator had it. Rebuilding the layout from
/// scratch (`scale = 1.0`, `transform = 0`, every connector listed) un-rotated portrait panels,
/// flattened a 2× scale, and switched a deliberately-dark head back on the moment a client
/// connected — while the code went to real trouble to preserve the refresh.
#[test]
fn a_kept_head_carries_its_pre_connect_scale_and_transform() {
// Rotated + 2×-scaled, exactly as it was before the virtual output appeared.
assert_eq!(
keep_head_layout(true, Some((2.0, 1)), Some((1.0, 0))),
Some((2.0, 1))
);
// Disabled on purpose (present pre-connect, carried by no logical monitor) — stays off.
assert_eq!(keep_head_layout(true, None, Some((1.0, 0))), None);
// Hotplugged inside our window: not in the snapshot at all, so keep it on at whatever
// Mutter derived rather than disabling a monitor the operator just plugged in.
assert_eq!(
keep_head_layout(false, None, Some((1.5, 2))),
Some((1.5, 2))
);
assert_eq!(keep_head_layout(false, None, None), Some((1.0, 0)));
// A junk scale would fail the WHOLE ApplyMonitorsConfig, taking the primary switch with it.
assert_eq!(keep_head_layout(true, Some((0.0, 3)), None), Some((1.0, 3)));
assert_eq!(
keep_head_layout(true, Some((f64::NAN, 0)), None),
Some((1.0, 0))
);
}
crates/pf-vdisplay/src/vdisplay/linux/portal_config.rs
+242 -6
View File
@@ -99,8 +99,42 @@ pub(crate) fn upsert(existing: &str, block: Block<'_>, key: &str, value: &str) -
/// Read `path`, set `key` in `block`, write it back — and back the original up ONCE, the first time
/// we touch a file we did not write. Returns `true` when the file changed (the caller restarts the
/// portal only then).
///
/// The read is matched EXPLICITLY, and only [`ErrorKind::NotFound`](std::io::ErrorKind::NotFound)
/// may mean "empty". This used to be `read_to_string(path).unwrap_or_default()`, which folded every
/// read failure into an empty string — and an empty string is the one input for which this function
/// destroys data: `upsert("")` yields a file holding ONLY our block, the backup below is skipped
/// because there is nothing to back up, and the write replaces the user's config. One non-UTF-8 byte
/// in a comment (a Latin-1 character, an 8-bit paste) or a transient EIO on an NFS/overlay config
/// dir was enough, and the result was exactly the silent, permanent loss this module exists to
/// prevent. A config we cannot read is a config we refuse to rewrite.
pub(crate) fn ensure_key(path: &Path, block: Block<'_>, key: &str, value: &str) -> Result<bool> {
let existing = std::fs::read_to_string(path).unwrap_or_default();
// Read BYTES: whether a backup is owed is a question about what is on disk, not about what
// decoded — and the decode failure below is itself one of the cases that must not be silent.
let raw = match std::fs::read(path) {
Ok(b) => Some(b),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => None,
Err(e) => {
return Err(e).with_context(|| {
format!(
"read {} (refusing to rewrite a portal config we could not read)",
path.display()
)
})
}
};
let existing = match &raw {
Some(bytes) => std::str::from_utf8(bytes)
.with_context(|| {
format!(
"{} is not UTF-8 — refusing to rewrite it (the one key we own is not worth \
losing the rest of the file for; fix or move the file and reconnect)",
path.display()
)
})?
.to_string(),
None => String::new(),
};
let updated = upsert(&existing, block, key, value);
if updated == existing {
return Ok(false);
@@ -108,9 +142,9 @@ pub(crate) fn ensure_key(path: &Path, block: Block<'_>, key: &str, value: &str)
if let Some(dir) = path.parent() {
std::fs::create_dir_all(dir).with_context(|| format!("mkdir {}", dir.display()))?;
}
// One-time backup. `create_new` makes this genuinely once: a later edit must not overwrite the
// user's ORIGINAL with our own previous output.
if !existing.is_empty() {
// One-time backup, of the bytes we actually read. `create_new` makes this genuinely once: a
// later edit must not overwrite the user's ORIGINAL with our own previous output.
if let Some(bytes) = raw.as_deref().filter(|b| !b.is_empty()) {
let backup = path.with_extension("punktfunk-backup");
match std::fs::OpenOptions::new()
.write(true)
@@ -119,7 +153,7 @@ pub(crate) fn ensure_key(path: &Path, block: Block<'_>, key: &str, value: &str)
{
Ok(mut f) => {
use std::io::Write;
let _ = f.write_all(existing.as_bytes());
let _ = f.write_all(bytes);
tracing::info!(
backup = %backup.display(),
"backed up the existing portal config before editing it"
@@ -133,10 +167,49 @@ pub(crate) fn ensure_key(path: &Path, block: Block<'_>, key: &str, value: &str)
),
}
}
std::fs::write(path, &updated).with_context(|| format!("write {}", path.display()))?;
write_atomic(path, updated.as_bytes())?;
Ok(true)
}
/// Replace `path`'s contents with `bytes` **atomically**: fill a temp file beside it, then rename
/// over it. `fs::write` truncates first and fills afterwards, so a crash, a full disk or a killed
/// host between the two leaves the user's config truncated — the same loss this module exists to
/// prevent, arrived at from the other side. The temp file goes in the SAME directory because a
/// rename is only atomic within one filesystem, and it inherits the original's permission bits so
/// an operator's 0600 config does not come back at the umask default.
fn write_atomic(path: &Path, bytes: &[u8]) -> Result<()> {
use std::io::Write;
let dir = path.parent().unwrap_or_else(|| Path::new("."));
let stem = path
.file_name()
.map(|n| n.to_string_lossy().into_owned())
.unwrap_or_else(|| "config".to_string());
// Per-process name: two hosts editing the same config must not fill one another's temp file.
let tmp = dir.join(format!(".{stem}.punktfunk-{}.tmp", std::process::id()));
let write = || -> Result<()> {
{
let mut f =
std::fs::File::create(&tmp).with_context(|| format!("create {}", tmp.display()))?;
f.write_all(bytes)
.with_context(|| format!("write {}", tmp.display()))?;
// The rename must not publish a name whose contents are still in the page cache only.
f.sync_all()
.with_context(|| format!("sync {}", tmp.display()))?;
} // closed before the rename — Windows is far happier renaming a file nobody holds open.
if let Ok(md) = std::fs::metadata(path) {
let _ = std::fs::set_permissions(&tmp, md.permissions());
}
std::fs::rename(&tmp, path)
.with_context(|| format!("rename {} -> {}", tmp.display(), path.display()))
};
let r = write();
if r.is_err() {
// Never leave a half-written dotfile beside the user's config.
let _ = std::fs::remove_file(&tmp);
}
r
}
#[cfg(test)]
mod tests {
use super::*;
@@ -237,3 +310,166 @@ mod tests {
);
}
}
/// [`ensure_key`] itself — the half that touches the user's disk.
///
/// The merge above was pinned by seven cases while the I/O wrapper around it, which is where the
/// destructive behaviour lives (the read, the once-only backup, the replacing write), had none. That
/// is backwards: `upsert` can at worst return a wrong string, `ensure_key` can delete a config.
/// Filesystem-only — no compositor, no portal — so these run on every platform, like the merge tests.
#[cfg(test)]
mod io_tests {
use super::*;
/// A scratch directory removed on drop. `tempfile` is deliberately not a dependency of this
/// crate; the temp-dir + pid + counter convention is the one `proc.rs`'s fixtures already use.
struct Scratch(std::path::PathBuf);
impl Scratch {
fn new(tag: &str) -> Self {
static N: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0);
let n = N.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
let dir = std::env::temp_dir()
.join(format!("pf-vd-portalcfg-{tag}-{}-{n}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).expect("scratch dir");
Self(dir)
}
fn path(&self, name: &str) -> std::path::PathBuf {
self.0.join(name)
}
}
impl Drop for Scratch {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
fn backup_of(p: &Path) -> std::path::PathBuf {
p.with_extension("punktfunk-backup")
}
/// The data-loss case. A config that cannot be decoded must be left EXACTLY as it is: the old
/// `unwrap_or_default()` turned it into an empty string, wrote a file holding only our block,
/// skipped the backup (nothing to back up, as far as it could tell) and returned `Ok(true)`.
#[test]
fn a_non_utf8_config_is_refused_not_replaced() {
let s = Scratch::new("nonutf8");
let p = s.path("config");
// A Latin-1 'ÿ' in a comment — the whole file is otherwise perfectly ordinary.
let raw: &[u8] = b"[screencast]\n# r\xffgler\nchooser_type=simple\noutput_name=DP-1\n";
std::fs::write(&p, raw).expect("seed");
let err = ensure_key(&p, Block::Ini("screencast"), "chooser_cmd", "cat x")
.expect_err("an unreadable config must not be rewritten");
assert!(
format!("{err:#}").contains("not UTF-8"),
"the error must name the real cause: {err:#}"
);
assert_eq!(
std::fs::read(&p).expect("still there"),
raw,
"byte-identical"
);
assert!(
!backup_of(&p).exists(),
"nothing was edited, so nothing is owed a backup"
);
}
/// The ordinary first-connect path: no file yet, so one is created — and there is no original
/// to preserve, so no backup is left lying beside it.
#[test]
fn a_missing_file_is_created_without_a_backup() {
let s = Scratch::new("missing");
let p = s.path("nested").join("config");
assert!(ensure_key(&p, Block::Ini("screencast"), "chooser_cmd", "cat x").expect("write"));
assert_eq!(
std::fs::read_to_string(&p).expect("created"),
"[screencast]\nchooser_cmd=cat x\n"
);
assert!(!backup_of(&p).exists());
}
/// `create_new` is what makes the backup once-only, and this is the invariant it buys: after a
/// second edit (a new `$XDG_RUNTIME_DIR`, so a new value) the backup must still hold the user's
/// PRISTINE file — not our own previous output.
#[test]
fn the_backup_holds_the_original_across_two_edits() {
let s = Scratch::new("backup");
let p = s.path("config");
let pristine = "[screencast]\nchooser_type=simple\noutput_name=DP-1\n";
std::fs::write(&p, pristine).expect("seed");
assert!(
ensure_key(&p, Block::Ini("screencast"), "chooser_cmd", "cat /run/a").expect("1st")
);
assert!(
ensure_key(&p, Block::Ini("screencast"), "chooser_cmd", "cat /run/b").expect("2nd")
);
assert_eq!(
std::fs::read_to_string(backup_of(&p)).expect("backup"),
pristine
);
let now = std::fs::read_to_string(&p).expect("edited");
assert!(
now.contains("chooser_cmd=cat /run/b"),
"the second value won"
);
assert!(
now.contains("output_name=DP-1"),
"the user's other keys survived"
);
}
/// Idempotence at the I/O level: an already-correct file is not rewritten and reports `false`,
/// because the caller RESTARTS the portal on `true` — a spurious `true` restarts xdpw/xdph on
/// every connect.
#[test]
fn an_unchanged_file_returns_false_and_does_not_rewrite() {
let s = Scratch::new("unchanged");
let p = s.path("config");
assert!(ensure_key(&p, Block::Ini("screencast"), "chooser_cmd", "cat x").expect("1st"));
let after_first = std::fs::read_to_string(&p).expect("written");
let mtime = std::fs::metadata(&p)
.and_then(|m| m.modified())
.expect("mtime");
assert!(
!ensure_key(&p, Block::Ini("screencast"), "chooser_cmd", "cat x").expect("2nd"),
"an unchanged config must report no change"
);
assert_eq!(
std::fs::read_to_string(&p).expect("still there"),
after_first
);
assert_eq!(
std::fs::metadata(&p)
.and_then(|m| m.modified())
.expect("mtime"),
mtime,
"the file must not have been touched at all"
);
}
/// The write publishes the WHOLE new file or nothing (temp + rename), and it leaves no debris
/// beside the config — a stray dotfile in `~/.config/hypr` is the kind of thing that outlives
/// several releases.
#[test]
fn the_write_is_atomic_and_leaves_no_temp_behind() {
let s = Scratch::new("atomic");
let p = s.path("config");
std::fs::write(&p, "[other]\nkeep=me\n").expect("seed");
assert!(ensure_key(&p, Block::Ini("screencast"), "chooser_cmd", "cat x").expect("write"));
let names: Vec<String> = std::fs::read_dir(&s.0)
.expect("dir")
.flatten()
.map(|e| e.file_name().to_string_lossy().into_owned())
.collect();
assert!(
!names.iter().any(|n| n.ends_with(".tmp")),
"temp file left behind: {names:?}"
);
assert!(std::fs::read_to_string(&p)
.expect("edited")
.contains("keep=me"));
}
}
crates/pf-vdisplay/src/vdisplay/linux/wlroots.rs
+149 -25
View File
@@ -40,7 +40,11 @@ fn chooser_file() -> String {
}
/// The chooser command xdpw runs via `/bin/sh -c`, reading stdout. The `|| echo` fallback keeps
/// plain portal capture (`--source portal`) working when no session has written the chooser file.
/// plain portal capture (`--source portal`) working when no session of ours is mid-handshake — it
/// is a GUESS at sway's own first headless output, right on a box whose sway loads the headless
/// backend with one output of its own and wrong (a cast of nothing) otherwise. It is reachable
/// again: the per-session file is removed with the handshake it steers ([`ChooserFile`]), so it no
/// longer sits there naming an output we have since unplugged.
fn chooser_cmd() -> String {
format!(
"cat {} 2>/dev/null || echo 'Monitor: HEADLESS-1'",
@@ -68,8 +72,14 @@ impl WlrootsDisplay {
/// wlroots/Sway is usable when the host runs inside a Sway session — signalled by `SWAYSOCK`
/// (the IPC socket `swaymsg create_output` needs). Cheap env check for the enumeration path.
///
/// Under [`crate::with_env_lock`]: this runs on a management worker (`/host/compositors` →
/// [`crate::available`]) concurrently with another connect's `apply_session_env`, which `set_var`s
/// — and, when no sway session is live, `remove_var`s — this very key. A glibc `getenv` racing a
/// `setenv` is the `environ` realloc data race ENV_LOCK exists for, and it is UB whichever key each
/// side names. No caller holds the lock (the mutex is not reentrant).
pub fn is_available() -> bool {
std::env::var_os("SWAYSOCK").is_some()
crate::with_env_lock(|| std::env::var_os("SWAYSOCK")).is_some()
}
impl VirtualDisplay for WlrootsDisplay {
@@ -86,13 +96,33 @@ impl VirtualDisplay for WlrootsDisplay {
}
fn create(&mut self, mode: Mode) -> Result<VirtualOutput> {
let before = output_names()
.context("swaymsg get_outputs (is the host inside the sway session env — SWAYSOCK?)")?;
swaymsg(&["create_output"])
.context("swaymsg create_output (sway needs the headless backend loaded)")?;
// The output appears synchronously in practice; poll briefly to be safe, and own it
// from here on so error unwinding unplugs it.
let output = OutputGuard(wait_new_output(&before, Duration::from_secs(5))?);
warn_topology_is_extend_only();
// Snapshot → create → identify, all under CREATE_LOCK. sway names the headless output
// itself (`HEADLESS-N`), so the only way to know which one is ours is "the name that was not
// there before" — and two concurrent creates each picking the other's output is a silent
// mis-capture, not a failure (mutter's TOPOLOGY_LOCK exists for exactly this class). The
// lock also gives the failure path somewhere safe to unplug from: the output already exists
// by the time `wait_new_output` can fail, and nothing else may have created one meanwhile.
let output = {
let _create = CREATE_LOCK.lock().unwrap_or_else(|e| e.into_inner());
let before = output_names().context(
"swaymsg get_outputs (is the host inside the sway session env — SWAYSOCK?)",
)?;
swaymsg(&["create_output"])
.context("swaymsg create_output (sway needs the headless backend loaded)")?;
// The output appears synchronously in practice; poll briefly to be safe, and own it
// from here on so error unwinding unplugs it.
match wait_new_output(&before, Duration::from_secs(5)) {
Ok(name) => OutputGuard(name),
Err(e) => {
// `create_output` reported success, so an output very probably exists — it just
// never showed up in time (or showed up a moment after we gave up). Unowned, it
// would sit in the operator's sway layout forever.
unplug_strays(&before);
return Err(e);
}
}
};
let name = output.0.clone();
// The client's exact mode (also the refresh clock that makes the output produce frames).
@@ -128,7 +158,7 @@ impl VirtualDisplay for WlrootsDisplay {
remote_fd: Some(fd),
preferred_mode: Some((mode.width, mode.height, mode.refresh_hz)),
keepalive: Box::new(Keepalive {
_stop: StopGuard(stop),
_stop: stop,
_output: output,
}),
// Owned (the compositor output is ours to tear down), but not registry-poolable: the
@@ -159,6 +189,52 @@ impl Drop for StopGuard {
}
}
/// Serializes **snapshot → `create_output` → identify-the-new-name**, process-wide. sway names its
/// headless outputs itself, so ownership is established by a before/after diff and two concurrent
/// creates would each adopt the other's output — which does not fail, it silently streams the wrong
/// one. Mutter's `TOPOLOGY_LOCK` is the same guard for the same reason; Hyprland needs none because
/// it lets us NAME the output (D6).
static CREATE_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
/// Unplug any headless output that appeared since `before` and that nothing owns — the cleanup for a
/// `create_output` whose output we could not identify in time. Only `HEADLESS-*` is touched: a
/// physical hotplug in the same window is the operator's, not ours, and `unplug` on a real connector
/// would take their screen away. Best-effort by construction, and it runs with [`CREATE_LOCK`] held
/// so nothing else in this process can have created the strays it sees.
fn unplug_strays(before: &[String]) {
let Ok(now) = output_names() else { return };
for name in now
.into_iter()
.filter(|n| n.starts_with("HEADLESS-") && !before.iter().any(|b| b == n))
{
match swaymsg(&["output", &name, "unplug"]) {
Ok(_) => tracing::warn!(output = %name, "unplugged a headless output we created but \
could not identify in time"),
Err(e) => tracing::warn!(output = %name, error = %format!("{e:#}"), "could not unplug \
the headless output left behind by a failed create"),
}
}
}
/// The configured [`crate::policy::Topology`] is not implemented on this backend — say so once per
/// create instead of leaving the management API's echo as the only signal that the pin was dropped
/// (sweep 13.18). sway's virtual output is always an EXTENSION: nothing here promotes it to primary
/// or disables the operator's heads.
fn warn_topology_is_extend_only() {
let topology = crate::effective_topology();
if !matches!(
topology,
crate::policy::Topology::Extend | crate::policy::Topology::Auto
) {
tracing::warn!(
?topology,
"wlroots: this backend implements EXTEND only — the headless output is added beside the \
operator's heads and nothing is promoted or disabled. Configure `topology: extend` to \
stop the console promising otherwise."
);
}
}
/// Owns the created headless output; dropping it unplugs it from sway.
struct OutputGuard(String);
@@ -171,15 +247,26 @@ impl Drop for OutputGuard {
}
}
/// Budget for one `swaymsg` call ([`crate::proc`]).
///
/// swaymsg is a CLIENT of the compositor it drives: against a wedged sway it blocks in its own
/// connect to the IPC socket and never returns — and these calls run on the session's stream thread,
/// whose only way to end a session is to return, so one hung query used to wedge the session
/// permanently. Generous next to a healthy call (single-digit milliseconds), and every call site
/// here already has a failed-query path, so a timeout lands on behaviour that already exists.
const SWAYMSG_BUDGET: Duration = Duration::from_secs(5);
/// Budget for the one-shot xdpw restart. `systemctl --user try-restart` waits for the unit's job to
/// settle, so it is the slowest helper on this path — and its result is already ignored.
const PORTAL_RESTART_BUDGET: Duration = Duration::from_secs(10);
/// Run `swaymsg -- <args>`, returning stdout (`--` so command tokens like `--custom` reach
/// sway instead of swaymsg's own getopt). swaymsg exits non-zero (with the error on stderr/
/// stdout) when the command fails, so checking the status covers `{"success": false}` too.
fn swaymsg(args: &[&str]) -> Result<String> {
let out = Command::new("swaymsg")
.arg("--")
.args(args)
.output()
.context("run swaymsg (is sway installed?)")?;
let out =
crate::proc::output_within(Command::new("swaymsg").arg("--").args(args), SWAYMSG_BUDGET)
.context("run swaymsg (is sway installed?)")?;
if !out.status.success() {
bail!(
"swaymsg {:?} failed: {}{}",
@@ -197,10 +284,11 @@ fn swaymsg(args: &[&str]) -> Result<String> {
/// *command*, which is right for `create_output` and wrong for a query — `-t` after `--` comes back
/// as `Unknown/invalid command '-t'` (caught on-glass writing the monitor enumeration).
fn swaymsg_query(kind: &str) -> Result<serde_json::Value> {
let out = Command::new("swaymsg")
.args(["-t", kind, "--raw"])
.output()
.context("run swaymsg (is sway installed?)")?;
let out = crate::proc::output_within(
Command::new("swaymsg").args(["-t", kind, "--raw"]),
SWAYMSG_BUDGET,
)
.context("run swaymsg (is sway installed?)")?;
if !out.status.success() {
bail!(
"swaymsg -t {kind} failed: {}",
@@ -230,13 +318,37 @@ fn output_names() -> Result<Vec<String>> {
/// handshake, not just the write, because the read happens inside it.
static SELECTION_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
/// The per-session chooser file, removed when the handshake it steers is over.
///
/// Its lifetime is the HANDSHAKE, not the session: xdpw reads it once, inside
/// [`select_and_cast`]'s critical section, and everything after that is the cast's own business.
/// Left behind (as it was) the stale `Monitor: HEADLESS-3` outlives the output `Drop` has since
/// unplugged, and it permanently shadows [`chooser_cmd`]'s `|| echo` fallback — so a later
/// `--source portal` capture with no session of ours running steers at a connector that is gone.
/// Tying removal to the CAST instead would be worse still: the file is one per user, so a session
/// ending hours later would delete a *sibling's* selection out from under its picker.
struct ChooserFile(String);
impl Drop for ChooserFile {
fn drop(&mut self) {
if let Err(e) = std::fs::remove_file(&self.0) {
if e.kind() != std::io::ErrorKind::NotFound {
tracing::debug!(path = %self.0, error = %e, "could not remove the xdpw chooser file");
}
}
}
}
/// Point xdpw's chooser at `output` and run the ScreenCast handshake, returning the portal fd +
/// node id and the guard that stops the cast. The caller must hold [`SELECTION_LOCK`].
fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, Arc<AtomicBool>)> {
fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, StopGuard)> {
ensure_xdpw_config()?;
let chooser = chooser_file();
std::fs::write(&chooser, format!("Monitor: {output}\n"))
.with_context(|| format!("write {chooser}"))?;
// Owned from the write on: every arm below (and every `?`) leaves the handshake, which is the
// only thing that reads it.
let _chooser = ChooserFile(chooser);
let (setup_tx, setup_rx) = std::sync::mpsc::channel::<Result<(OwnedFd, u32), String>>();
let stop = Arc::new(AtomicBool::new(false));
let stop_thread = stop.clone();
@@ -244,8 +356,16 @@ fn select_and_cast(output: &str, hw_cursor: bool) -> Result<(OwnedFd, u32, Arc<A
.name("punktfunk-wlr-cast".into())
.spawn(move || portal_thread(setup_tx, stop_thread, hw_cursor))
.context("spawn wlroots portal thread")?;
// Built BEFORE the wait so EVERY error arm below sets the flag on its way out — as Mutter's
// `create` does. Returning the bare `Arc` and letting the CALLER wrap it left the two failure
// arms dropping an un-set flag: the thread's `send` can still LAND in the queue in the window
// between `recv_timeout` giving up and `setup_rx` being dropped, so it reports success and then
// parks forever on `while !stop`, holding a live ScreenCast session, its zbus connection, an
// `OwnedFd` and a 2-worker tokio runtime — one more set per slow-portal connect, for the host's
// lifetime, against an output that no longer exists.
let guard = StopGuard(stop);
match setup_rx.recv_timeout(Duration::from_secs(20)) {
Ok(Ok((fd, node_id))) => Ok((fd, node_id, stop)),
Ok(Ok((fd, node_id))) => Ok((fd, node_id, guard)),
Ok(Err(e)) => bail!("ScreenCast portal on {output} failed: {e}"),
Err(_) => bail!("timed out waiting for the ScreenCast portal on {output}"),
}
@@ -266,7 +386,7 @@ pub(crate) fn stream_existing_output(
Ok(crate::mirror::MirrorStream {
node_id,
remote_fd: Some(fd),
keepalive: Box::new(StopGuard(stop)),
keepalive: Box::new(stop),
})
}
@@ -374,9 +494,13 @@ fn ensure_xdpw_config() -> Result<()> {
return Ok(());
}
tracing::info!(path = %path.display(), "pointed xdg-desktop-portal-wlr at the managed output chooser");
let _ = Command::new("systemctl")
.args(["--user", "try-restart", "xdg-desktop-portal-wlr.service"])
.status();
// Bounded: `systemctl --user` blocks on the user manager's job queue, and this runs on the
// session's stream thread. Its result was already ignored — a timeout just means the portal
// picks the new config up whenever it next starts.
let _ = crate::proc::status_within(
Command::new("systemctl").args(["--user", "try-restart", "xdg-desktop-portal-wlr.service"]),
PORTAL_RESTART_BUDGET,
);
Ok(())
}