capture: probe the display before starting capture - #34
Open
josiahbryan wants to merge 2 commits into
Open
Conversation
StartCapture picked a backend by checking only whether DISPLAY or
WAYLAND_DISPLAY was non-empty. A variable that is set is not the same as
a display that exists, and the difference is silent: capture starts, the
GStreamer pipeline runs, and the receiver shows a black screen.
The case that motivated this is a long-lived process that outlives the
desktop session it was started from. A lingering systemd user service is
the clearest example -- it inherits DISPLAY, WAYLAND_DISPLAY and
XAUTHORITY from the graphical login and keeps them after logout, when
the compositor is gone and its sockets have been removed. Observed on
Ubuntu 26.04 / GNOME Wayland: after `loginctl terminate-session`, the
service environment still reads
DISPLAY=:0
WAYLAND_DISPLAY=wayland-0
XAUTHORITY=/run/user/1000/.mutter-Xwaylandauth.0ED6Q3 (deleted)
while /tmp/.X11-unix/X0 and $XDG_RUNTIME_DIR/wayland-0 no longer exist.
Both variables are set, so the existing "no display server detected"
guard cannot fire.
Measured on that box, before this change:
- X11 with a stale DISPLAY logged "screen capture started" and
"mirror session ready", then failed later with "capture process
exited unexpectedly (EOF)", which names the wrong culprit.
- Wayland with a stale WAYLAND_DISPLAY was worse: it blocked in the
xdg-desktop-portal call and produced no further output and no error
at all until killed.
Probe the endpoint instead of trusting the variable: resolve DISPLAY to
its unix socket or TCP address the way Xlib does, resolve WAYLAND_DISPLAY
against XDG_RUNTIME_DIR, and dial it with a short timeout. A stale
variable now fails immediately, naming what was tried and why it did not
answer.
This deliberately checks reachability only, not authentication. A
reachable server that rejects the cookie still fails later in GStreamer,
with GStreamer's own message.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
x11Endpoint is the part of the display probe with a contract worth
pinning: the unix-socket-or-TCP rule, the screen suffix, the optional
protocol prefix, and the display values that must be rejected rather
than guessed at. A wrong endpoint here reintroduces exactly what the
probe exists to prevent -- a capture started against nothing.
Writing those tests turned up a bug in the probe itself. An IPv6 literal
that arrives already bracketed, as in DISPLAY=[::1]:0, was handed to
JoinHostPort with its brackets still attached. JoinHostPort brackets any
host containing a colon, so the result was an undialable
[[::1]]:6000
Strip the brackets before joining. Removing that strip makes the IPv6
case fail again, so the test covers the fix rather than merely passing
alongside it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
StartCapturechooses a backend by checking whetherDISPLAYorWAYLAND_DISPLAYis non-empty. A variable that is set is not a display that exists, and the difference is silent: capture starts, the pipeline runs, and the receiver shows a black screen.The case that motivated this is a long-lived process outliving the desktop session it was started from. A lingering systemd user service is the clearest example — it inherits
DISPLAY,WAYLAND_DISPLAYandXAUTHORITYat graphical login and keeps them after logout, when the compositor is gone and its sockets have been removed.Observed on Ubuntu 26.04 / GNOME Wayland. After
loginctl terminate-sessionthe service environment still readswhile
/tmp/.X11-unix/X0and$XDG_RUNTIME_DIR/wayland-0no longer exist. Both variables are set, so the existing "no display server detected" guard cannot fire.Measured on that box, before this change:
DISPLAYloggedscreen capture startedandmirror session ready, then failed later withcapture process exited unexpectedly (EOF)— which names the wrong culprit.WAYLAND_DISPLAYwas worse: it blocked in thexdg-desktop-portalcall and produced no output and no error at all until killed.The change
Probe the endpoint instead of trusting the variable. Resolve
DISPLAYto its unix socket or TCP address the way Xlib does, resolveWAYLAND_DISPLAYagainstXDG_RUNTIME_DIR, and dial it with a short timeout. A stale variable now fails immediately, naming what was tried and why it did not answer.This deliberately checks reachability only, not authentication. A reachable server that rejects the cookie still fails later in GStreamer, with GStreamer's own message. I did not want the probe to start duplicating auth logic it would then have to keep in sync.
Tests, and a bug they found
The second commit tests
x11Endpoint— the unix-socket-or-TCP rule, the screen suffix, the optional protocol prefix, and the values that must be rejected rather than guessed at. A wrong endpoint reintroduces exactly what the probe exists to prevent.Writing them turned up a bug in my own first commit. An IPv6 literal that arrives already bracketed, as in
DISPLAY=[::1]:0, was handed toJoinHostPortwith its brackets still on.JoinHostPortbrackets any host containing a colon, so the result was an undialable[[::1]]:6000. The fix strips them first; removing that strip makes the IPv6 case fail again, so the test covers the fix rather than passing alongside it.I've left it as two commits so that's visible rather than tidied away.
gofmt,go vetandgo test ./...are clean, verified on a clean checkout of the commit rather than a working tree.Scope
The probe only runs where the code already decided to use a display, so it adds a dial and a timeout to paths that were about to fail anyway. It does not change behaviour when the display is reachable.