Connecting from the 0.22.0 Windows client bounces straight back to the host list, on every host. The shell is fine; the binary it spawns is not.b0ea1e6bwas a `clients/linux` change that also dropped a verbatim copy of the GTK shell into `clients/session/src/` — app.rs, cli.rs, the four ui_*.rs, shortcuts.rs, spawn.rs — and, fatally, OVERWROTE `clients/session/src/main.rs` with the shell's. That file is `[[bin]] punktfunk-session`. So the binary every shell execs for a stream stopped being the Vulkan session and became the GTK shell: - Windows: the shell's `#[cfg(not(target_os = "linux"))]` arm — print "punktfunk-client is Linux-only" to stderr, `exit(2)`. It never writes a single line of the stdout contract, so the shell sees EOF with no `ready`, no `error`, no `ended`, and returns to the host list with a BLANK banner. Exactly the report. - Linux: `app::run()` sees `--connect` in argv and calls `exec_session()`, which execs `punktfunk-session` — itself. A stream is an exec loop. CI could not catch it. The Windows leg of the clobbered file is a three-line stub that compiles perfectly; `cargo build -p punktfunk-client-session` stayed green while building the wrong program. Only running it fails, and nothing runs it.61bdf11ethen read the 327 E0433s on the Linux leg as a missing-manifest bug and declared gtk4/libadwaita/relm4 on this crate. That fixed the build of the wrong file and cemented the clobber. Both go: the copy is deleted, `main.rs` is restored frombf981027(the last commit that touched the real one), and the manifest loses the GTK block plus the gresource build-dep and `data/` that arrived with944c03ddto feed it. serde_json returns to `optional`/`ui`, which is what it always was — the reason `--no-default-features` didn't compile was cli.rs, and cli.rs was never ours. Also closes the hole that made this silent. `SpawnEvent::Exited` now carries the child's exit code, and a child that exits nonzero having said NOTHING gets a banner naming that code instead of an empty string. Code 0 (stream window closed) and -1 (our own Disconnect/Cancel kill) stay silent, as before. A wrong or crashing session binary is now a legible failure rather than a connect that quietly does nothing. Verified. Windows x86_64 on the CI runner: check, clippy -D warnings, rustfmt, tests (5 passed, incl. the new one) all green, and the rebuilt punktfunk-session.exe answers `--connect` with `{"error":"presenter: SDL window: …"}` — the real contract — where 0.22.0's answered with nothing. Linux amd64 in the CI image: check with default AND --no-default-features, clippy -D warnings, rustfmt, tests green, and the built binary emits `{"error":"presenter: SDL video: …"}` + exit 4 instead of exec-looping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
297 lines
12 KiB
Rust
297 lines
12 KiB
Rust
//! The shell↔session handoff: streams run in the spawned `punktfunk-session` Vulkan
|
|
//! binary (session-always, mirroring the GTK shell's `clients/linux/src/spawn.rs`). This
|
|
//! module owns the child's lifecycle plumbing — spawned with CREATE_NO_WINDOW (the
|
|
//! session keeps the console subsystem for its stdout contract; without the flag a GUI
|
|
//! parent would pop a console window), its stdout contract parsed into typed
|
|
//! [`SpawnEvent`]s a reader thread hands to the app's navigation closure: spinner until
|
|
//! `{"ready":true}`, banner from the `{"error"|"ended": …}` line, `trust_rejected`
|
|
//! routed to the re-pair PIN ceremony, `stats:` lines to the session status page.
|
|
|
|
use std::io::BufRead as _;
|
|
use std::process::{Child, Command, Stdio};
|
|
use std::sync::{Arc, Mutex};
|
|
|
|
/// One parsed event from the session child.
|
|
pub(crate) enum SpawnEvent {
|
|
/// The child presented its first frame (its window is up and streaming).
|
|
Ready,
|
|
/// One `stats:` line, already human-formatted by the session (per 1 s window).
|
|
Stats(String),
|
|
/// The child exited (stdout EOF + reap; a kill lands here too). `error`/`ended`
|
|
/// carry the contract lines seen on the way out, when any — routing keys off those,
|
|
/// which say strictly more than a number. `code` is the process exit status (-1 = no
|
|
/// code, i.e. killed) and exists for the case where there were NO lines at all: a
|
|
/// child that dies before it can speak the contract would otherwise be indistinguishable
|
|
/// from a clean user-initiated quit, and the shell would bounce to the host list with a
|
|
/// blank banner. That is exactly how the 0.22.0 session-binary regression presented.
|
|
Exited {
|
|
error: Option<(String, bool)>,
|
|
ended: Option<String>,
|
|
code: i32,
|
|
},
|
|
}
|
|
|
|
/// Kills the spawned session child (the Disconnect button, request-access Cancel). Safe
|
|
/// to call any time; a child that already exited is a no-op. A FRESH handle is installed
|
|
/// per spawn (`Shared::session`) so a stale handle can never kill a newer session.
|
|
#[derive(Clone, Default)]
|
|
pub(crate) struct SessionChild(Arc<Mutex<Option<Child>>>);
|
|
|
|
impl SessionChild {
|
|
pub(crate) fn kill(&self) {
|
|
if let Some(child) = self.0.lock().unwrap().as_mut() {
|
|
let _ = child.kill();
|
|
}
|
|
}
|
|
|
|
/// Whether a spawned child is currently live (spawned and not yet reaped by its
|
|
/// reader). The probe sweep pauses while one runs — the shell is hidden, and probing
|
|
/// the host we're streaming from is just noise.
|
|
pub(crate) fn is_running(&self) -> bool {
|
|
self.0.lock().unwrap().is_some()
|
|
}
|
|
}
|
|
|
|
/// One parsed stdout line of the session contract; `None` for anything unrecognized.
|
|
enum ChildLine {
|
|
Ready,
|
|
Error { msg: String, trust_rejected: bool },
|
|
Ended(String),
|
|
Stats(String),
|
|
}
|
|
|
|
fn parse_line(line: &str) -> Option<ChildLine> {
|
|
if let Some(stats) = line.strip_prefix("stats: ") {
|
|
return Some(ChildLine::Stats(stats.to_string()));
|
|
}
|
|
let v: serde_json::Value = serde_json::from_str(line).ok()?;
|
|
if v.get("ready").and_then(|r| r.as_bool()) == Some(true) {
|
|
return Some(ChildLine::Ready);
|
|
}
|
|
if let Some(msg) = v.get("error").and_then(|m| m.as_str()) {
|
|
return Some(ChildLine::Error {
|
|
msg: msg.to_string(),
|
|
trust_rejected: v.get("trust_rejected").and_then(|t| t.as_bool()) == Some(true),
|
|
});
|
|
}
|
|
if let Some(msg) = v.get("ended").and_then(|m| m.as_str()) {
|
|
return Some(ChildLine::Ended(msg.to_string()));
|
|
}
|
|
None
|
|
}
|
|
|
|
/// The banner for a child that exited having said NOTHING on stdout — no `ready`, no
|
|
/// `error`, no `ended`. `None` keeps the silent return the UI has always given a clean
|
|
/// quit: code 0 is the user closing the stream window, and -1 is our own Disconnect/Cancel
|
|
/// kill (no exit code). Anything else is the session dying before it could speak its
|
|
/// contract — a missing runtime DLL, a crash, or the wrong binary sitting next to the
|
|
/// shell — and reporting the code is the difference between a diagnosable failure and a
|
|
/// connect that silently drops back to the host list.
|
|
pub(crate) fn silent_exit_banner(code: i32) -> Option<String> {
|
|
(code != 0 && code != -1).then(|| {
|
|
format!("The session didn't start (punktfunk-session exited with code {code}). Check the client log.")
|
|
})
|
|
}
|
|
|
|
/// The session binary: installed next to the shell (the MSIX layout and dev
|
|
/// `target\…` runs both land on the sibling), else `PATH`.
|
|
pub(crate) fn session_binary() -> std::path::PathBuf {
|
|
if let Ok(exe) = std::env::current_exe() {
|
|
let sibling = exe.with_file_name("punktfunk-session.exe");
|
|
if sibling.exists() {
|
|
return sibling;
|
|
}
|
|
}
|
|
"punktfunk-session".into()
|
|
}
|
|
|
|
/// Spawn the session binary for a connect with `fp_hex` pinned and feed its lifecycle to
|
|
/// `on_event` from a reader thread. The child is parked in `slot` so Disconnect/Cancel
|
|
/// can kill it. `fullscreen` starts the stream window fullscreen (the Settings "Start
|
|
/// streams fullscreen" toggle); `launch` carries a library title id for the host to
|
|
/// launch during the handshake. `Err` = the spawn itself failed (binary missing?) —
|
|
/// surfaced as a connect error by the caller.
|
|
#[allow(clippy::too_many_arguments)] // one cohesive spawn spec (session_params precedent)
|
|
pub(crate) fn spawn_session(
|
|
addr: &str,
|
|
port: u16,
|
|
fp_hex: &str,
|
|
connect_timeout_secs: u64,
|
|
fullscreen: bool,
|
|
launch: Option<&str>,
|
|
profile: Option<&str>,
|
|
slot: SessionChild,
|
|
on_event: impl FnMut(SpawnEvent) + Send + 'static,
|
|
) -> Result<(), String> {
|
|
let mut cmd = Command::new(session_binary());
|
|
cmd.arg("--connect")
|
|
.arg(format!("{addr}:{port}"))
|
|
.arg("--fp")
|
|
.arg(fp_hex)
|
|
.arg("--connect-timeout")
|
|
.arg(connect_timeout_secs.to_string());
|
|
if fullscreen {
|
|
cmd.arg("--fullscreen");
|
|
}
|
|
if let Some(id) = launch {
|
|
cmd.arg("--launch").arg(id);
|
|
}
|
|
// Only a ONE-OFF pick rides the flag: without it the session resolves the host's own
|
|
// binding through the same helper this shell would have used, so the two can't disagree.
|
|
if let Some(reference) = profile {
|
|
cmd.arg("--profile").arg(reference);
|
|
}
|
|
add_window_pos(&mut cmd);
|
|
spawn_with(cmd, &format!("{addr}:{port}"), slot, on_event)
|
|
}
|
|
|
|
/// Spawn the session binary in `--browse` mode: the console (gamepad) library for a
|
|
/// PAIRED host, in the session window — launches run as streams in that same window.
|
|
/// The same stdout contract as a connect (`--json-status`): `ready` when the library
|
|
/// window presents, `error` on a failed start, EOF on quit.
|
|
pub(crate) fn spawn_browse(
|
|
target: Option<(&str, u16)>,
|
|
fullscreen: bool,
|
|
slot: SessionChild,
|
|
on_event: impl FnMut(SpawnEvent) + Send + 'static,
|
|
) -> Result<(), String> {
|
|
let mut cmd = Command::new(session_binary());
|
|
cmd.arg("--browse");
|
|
// A target opens straight into that host's library; bare `--browse` opens the console's
|
|
// OWN host view (discovery, pairing, settings, Wake-on-LAN) — the couch equivalent of
|
|
// the shell's hosts page.
|
|
if let Some((addr, port)) = target {
|
|
cmd.arg(format!("{addr}:{port}"));
|
|
}
|
|
cmd.arg("--json-status");
|
|
if fullscreen {
|
|
cmd.arg("--fullscreen");
|
|
}
|
|
add_window_pos(&mut cmd);
|
|
let label = target.map_or_else(|| "console".to_string(), |(a, p)| format!("{a}:{p}"));
|
|
spawn_with(cmd, &label, slot, on_event)
|
|
}
|
|
|
|
/// Hand the shell window's position to the child (`--window-pos`) so the session window
|
|
/// opens on the same monitor, where the shell is — the hide/restore handoff then reads as
|
|
/// one window changing content instead of a window jumping displays.
|
|
fn add_window_pos(cmd: &mut Command) {
|
|
if let Some((x, y)) = crate::shell_window::position() {
|
|
cmd.arg("--window-pos").arg(format!("{x},{y}"));
|
|
}
|
|
}
|
|
|
|
/// The shared spawn + stdout-contract reader behind [`spawn_session`]/[`spawn_browse`].
|
|
fn spawn_with(
|
|
mut cmd: Command,
|
|
host_label: &str,
|
|
slot: SessionChild,
|
|
mut on_event: impl FnMut(SpawnEvent) + Send + 'static,
|
|
) -> Result<(), String> {
|
|
use std::os::windows::process::CommandExt as _;
|
|
const CREATE_NO_WINDOW: u32 = 0x0800_0000;
|
|
|
|
cmd.stdin(Stdio::null())
|
|
.stdout(Stdio::piped())
|
|
// Piped through the log tee: dev-terminal runs keep the interleaved stderr they always
|
|
// had, and GUI runs — which have no console — finally keep the session's whole
|
|
// receive/decode/present log in the client log file.
|
|
.stderr(Stdio::piped())
|
|
.creation_flags(CREATE_NO_WINDOW);
|
|
let mut child = cmd
|
|
.spawn()
|
|
.map_err(|e| format!("couldn't start punktfunk-session: {e}"))?;
|
|
tracing::info!(host = %host_label, "session binary spawned");
|
|
|
|
if let Some(stderr) = child.stderr.take() {
|
|
crate::logfile::forward_child_stderr(stderr);
|
|
}
|
|
let stdout = child.stdout.take().expect("piped stdout");
|
|
// Park the child where the kill handle (and the reader, for the final reap) reach it.
|
|
*slot.0.lock().unwrap() = Some(child);
|
|
|
|
std::thread::Builder::new()
|
|
.name("punktfunk-session-io".into())
|
|
.spawn(move || {
|
|
let mut error: Option<(String, bool)> = None;
|
|
let mut ended: Option<String> = None;
|
|
for line in std::io::BufReader::new(stdout).lines() {
|
|
let Ok(line) = line else { break };
|
|
match parse_line(&line) {
|
|
Some(ChildLine::Ready) => on_event(SpawnEvent::Ready),
|
|
Some(ChildLine::Stats(s)) => on_event(SpawnEvent::Stats(s)),
|
|
Some(ChildLine::Error {
|
|
msg,
|
|
trust_rejected,
|
|
}) => error = Some((msg, trust_rejected)),
|
|
Some(ChildLine::Ended(msg)) => ended = Some(msg),
|
|
None => {}
|
|
}
|
|
}
|
|
// EOF — reap the child (killed-by-Disconnect lands here too; -1 = no code).
|
|
let code = slot
|
|
.0
|
|
.lock()
|
|
.unwrap()
|
|
.take()
|
|
.and_then(|mut c| c.wait().ok())
|
|
.and_then(|s| s.code())
|
|
.unwrap_or(-1);
|
|
tracing::info!(code, "session binary exited");
|
|
on_event(SpawnEvent::Exited { error, ended, code });
|
|
})
|
|
.map_err(|e| format!("session reader thread: {e}"))?;
|
|
Ok(())
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
#[test]
|
|
fn parses_the_stdout_contract() {
|
|
assert!(matches!(
|
|
parse_line("{\"ready\":true}"),
|
|
Some(ChildLine::Ready)
|
|
));
|
|
match parse_line("{\"error\":\"no route\",\"trust_rejected\":false}") {
|
|
Some(ChildLine::Error {
|
|
msg,
|
|
trust_rejected,
|
|
}) => {
|
|
assert_eq!(msg, "no route");
|
|
assert!(!trust_rejected);
|
|
}
|
|
_ => panic!("error line"),
|
|
}
|
|
match parse_line("{\"error\":\"pin\",\"trust_rejected\":true}") {
|
|
Some(ChildLine::Error { trust_rejected, .. }) => assert!(trust_rejected),
|
|
_ => panic!("trust line"),
|
|
}
|
|
match parse_line("{\"ended\":\"Host ended the session\"}") {
|
|
Some(ChildLine::Ended(m)) => assert_eq!(m, "Host ended the session"),
|
|
_ => panic!("ended line"),
|
|
}
|
|
// Stats lines become Stats events; stray output never becomes an event.
|
|
match parse_line("stats: 1280\u{00D7}800@60 \u{00B7} 60 fps") {
|
|
Some(ChildLine::Stats(s)) => assert!(s.starts_with("1280")),
|
|
_ => panic!("stats line"),
|
|
}
|
|
assert!(parse_line("").is_none());
|
|
assert!(parse_line("{\"other\":1}").is_none());
|
|
}
|
|
|
|
#[test]
|
|
fn a_silent_failing_exit_is_never_blank() {
|
|
// Clean quit (stream window closed) and our own kill stay silent.
|
|
assert!(silent_exit_banner(0).is_none());
|
|
assert!(silent_exit_banner(-1).is_none());
|
|
// A child that died without speaking the contract names its code — the 0.22.0
|
|
// regression (a stub session binary exiting 2) showed as a blank bounce to the
|
|
// host list precisely because nothing filled this in.
|
|
let banner = silent_exit_banner(2).expect("failing exit must say something");
|
|
assert!(banner.contains('2'), "{banner}");
|
|
assert!(silent_exit_banner(101).is_some());
|
|
}
|
|
}
|