Addresses fufesou's #580 re-review:
- The child runs in its own process group and the deadline kills the
whole group, so a hung loginctl or a leaked grandchild cannot survive
the child or keep the pipes open past the reads. Verified: a
deliberately leaked sleep grandchild no longer outlives the probe.
- The probe parses compositor-controlled data, so before touching the
socket the child drops to the runtime directory's owner and refuses
to probe at all if the drop fails, rather than parsing untrusted
bytes as root. loginctl still runs as root first, since finding the
seat needs it.
- first_buffered_line now distinguishes an inspected-but-empty pipe
(WouldBlock) from an uninspectable one (fcntl or read error): only a
real absence of the handshake latches the binary unsupported, an
inspection failure does not.
- rustfmt-clean.
A binary that does not dispatch the probe argument runs its normal
startup; a long-running one outlives the deadline and was killed before
the handshake check could run, so PROBE_UNSUPPORTED never latched and
every enumeration cycle spawned a full consumer process again. The
timeout path now judges the child by what it already wrote: a real
probe prints the magic line first and flushes, so its absence after a
whole deadline means this is not a probe. Only the buffered bytes are
read, non-blocking, because an EOF-seeking read could hang on a
grandchild that inherited the write end of the pipe.
Also report the exit status when a failed child left stderr empty,
which panic=abort and signals do, and name the malformed-list case in
the deserialization error.
Two review findings:
- The release profile builds with panic=abort, so the in-thread isolation
was an illusion: an sctk panic on malformed bytes from a scanned socket
aborted the whole server before ProbeBusyGuard could run. The probe now
runs in a child process spawned from the current executable, which the
consumer binary dispatches to wayland_display_probe_child_main before
any other startup work. A panic there kills only the child, and the
deadline now kills the child instead of leaking a blocked thread.
- seat0_runtime_dir ran loginctl through Command::output with no bound,
before the worker and its timeout existed, while the caller held the
DISPLAYS lock. The lookup now runs inside the child, under the same
two second deadline as everything else.
A binary that does not dispatch the probe arg fails a magic line
handshake and the probe latches off for the process lifetime, so the
fallback degrades to the pre-fallback behavior instead of spawning a
full consumer process per enumeration cycle. The consumer wiring is one
early dispatch in core_main:
#[cfg(target_os = "linux")]
if std::env::args().nth(1).as_deref()
== Some(hbb_common::platform::linux::WAYLAND_DISPLAY_PROBE_ARG)
{
hbb_common::platform::linux::wayland_display_probe_child_main();
}
The fallback was silent on success, so a host where it engaged looked identical to one
where it was never reached. Debug level: it runs on a cache miss, not per frame.
Answers the review on this PR. The fallback kept three states that could not
self-heal, and two of them came from the same place: it trusted the environment and
it had no deadline.
B1, no deadline on any part of the new path while the caller holds a process-wide
lock: `connect(2)` parks on a full backlog and sctk's roundtrip polls with no
deadline, so a socket that accepts and never speaks wayland stalls the display
service, the six drm_capturer call sites and the flutter_ffi SyncReturn. The probe
now runs on its own thread with a 2 s deadline, under the 3 s the uinput caller
already budgets, and only one probe can be in flight. That also keeps sctk's
`panic!`/`todo!` on malformed output events off the thread holding the lock, so it
can no longer poison that mutex, and a `Builder` is used because `thread::spawn`
panics when a thread cannot be created.
B2, a newly reachable empty `Ok` latched into the caller's cache for the process
lifetime: an empty list is now an error, so the existing no-cache path still
applies and the next poll retries.
B3, the `WAYLAND_SOCKET` half of the guard could not fire, because
`connect_to_env` removes that variable on its success and bad-fd paths and this is
re-entered every ~1.5 s. Both variables are read at the call site before
connecting, and the answer is latched: a consumed variable cannot turn a process
that was pointed at a compositor into one free to look for another.
C4 and S1 have the same fix: the directory is `/run/user/<uid>` of the active seat0
session rather than `XDG_RUNTIME_DIR`, so no environment value reaches a connect in
a process that can be root, and it works in the default build, which is given no
such variable (C3). The candidates are scanned rather than guessed, since
`wl_display_add_socket_auto` takes the first FREE name up to `wayland-32` and a
greeter accumulates leftovers; that drops the `path.exists()` pre-stat and its
TOCTOU window with it (C7), and empty values are no longer read as names (C6).
C2, the first socket that accepted used to win unconditionally: the loop now
carries on to the next candidate when a socket connects but fails the protocol or
reports no outputs.
C8, `registry_handlers!()` was empty, so a `wl_output` advertised between the
registry snapshot and the roundtrip was dropped and never reached `outputs()`.
C5 is real and is not fixable here: `get_primary_monitor` spawning a timeout-less
xrandr inside the held lock is in the consumer, and I will send it there.
Review of the first version, all three correct.
The important one: `connect_to_env()` honours `WAYLAND_DISPLAY` and `WAYLAND_SOCKET`,
and the fallback ignored that choice. If an explicit endpoint was named and failed,
this could attach to a *different* compositor and read output positions from the wrong
display. It now runs only when neither variable is set, which is also the only case it
was ever justified by.
`XDG_RUNTIME_DIR` is read with `var_os` and rejected when relative, since `.` or
`../tmp` would probe against the working directory. And every failed probe is kept
instead of the last one overwriting the first, so an error naming both sockets says
what happened to both.
The comment is three lines now: the reasoning belongs here, in the commit.
`get_wayland_displays` gives up as soon as `Connection::connect_to_env()` fails,
which is any process that was not handed `WAYLAND_DISPLAY`. A login screen is
the case that matters: rustdesk starts the greeter's `--server` without the
compositor variables on purpose, and the desktop layout is then unavailable for
the whole session even though the compositor is running and its socket sits in
the runtime directory of the very user that process runs as.
Measured at an sddm Plasma Wayland greeter: `/run/user/112/wayland-0` exists,
`connect(2)` to it as that user succeeds, and the registry advertises
`wl_output` and `wl_seat`. With this fallback the greeter reads the same output
layout any other session does.
Strictly a fallback after `connect_to_env` has already failed, so it cannot
change a host where that succeeds; where there is no compositor at all the
socket is simply absent and it fails exactly as before.
1. Add filed `scale` in `message SwitchDisplay`(`message.proto`).
2. Add new function to get all display info.
3. Add help link of KDE Plasma Wayland.
Signed-off-by: fufesou <linlong1266@gmail.com>
If a user for reasons start the program as user unit (instead of
as a system unit or in a session), /proc/self/sessionid may result
in the inferred session type being "unspecified" and prevent
XDG_SESSION_TYPE from being leveraged (and its "x11" fallback).
With modern systemd versions, there is a manager class session,
which is of type "unspecified", for each logged-in / lingering
user. Such session should be ignored when attempt to find an
appropriate session from all (non-seated) sessions, otherwise it
will prevent XDG_SESSION_TYPE (and its "x11" fallback) from being
leveraged.
Mariano Abad
fufesou
Tom Yan
21pages