fix(session/gamelease): four things the first pass got wrong
Found reviewing the riskiest paths of the previous commit. - An install dir of `/games/x` matched a process running out of `/games/xyz`. The image-path check was already component-wise and fine; the command-line check compared raw bytes, so it needed the separator. Two library folders where one name prefixes the other is not a contrived case. - Ending a nested game force-releases kept displays, which is not per-display — so with another client streaming it could retire a display that was never ours. Skip it while anyone else has a live session: the failure mode becomes a game that keeps running, which is the default everywhere else anyway. - A lease nothing polls (no detect signals, or a platform without a matcher yet) sat at "launching" forever in the console. Report it as running: the host did just launch it, and with no watcher it is claiming nothing about the exit. - A game ended after its session was already gone reported no `game.exited` at all — its watcher had stopped with the session, so nothing was left to emit it. That is every grace expiry and every `POST /game/end`, i.e. exactly the ones an operator most wants to see. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -335,6 +335,13 @@ pub fn open(req: LeaseRequest, on_exit: OnExit) -> GameLease {
|
||||
}
|
||||
|
||||
let watcher = spawn_watcher(shared.clone(), child, on_exit);
|
||||
if watcher.is_none() {
|
||||
// Nothing is polling this lease (no signals to poll, or a platform without a matcher yet), so
|
||||
// its state will never advance on its own. Report it as running rather than leaving the
|
||||
// console showing "launching" forever: the host did just launch it, and with no watcher it is
|
||||
// making no claim about noticing the exit. The row disappears when the session ends.
|
||||
shared.set_state(GameState::Running);
|
||||
}
|
||||
GameLease { shared, watcher }
|
||||
}
|
||||
|
||||
@@ -564,9 +571,20 @@ pub fn terminate(shared: Arc<LeaseShared>, why: &'static str) {
|
||||
}
|
||||
tracing::info!(title = %shared.game.title, reason = why, "ending the launched game");
|
||||
let name = "pf1-gameterm".to_string();
|
||||
let _ = std::thread::Builder::new()
|
||||
.name(name)
|
||||
.spawn(move || terminate_blocking(&shared));
|
||||
let _ = std::thread::Builder::new().name(name).spawn(move || {
|
||||
terminate_blocking(&shared);
|
||||
// A lease whose session is still up has a live watcher, which reports the exit itself
|
||||
// (with the right reason) once it confirms the game is gone. A lease whose session has
|
||||
// already ended — every grace expiry and every `POST /game/end` — has no watcher left, so
|
||||
// this is the only place its exit can be reported from.
|
||||
if shared.cancel.load(Ordering::Relaxed) {
|
||||
shared.set_state(GameState::Exited);
|
||||
crate::events::emit(crate::events::EventKind::GameExited {
|
||||
game: game_event_ref(&shared),
|
||||
reason: crate::events::GameEndReason::Terminated,
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/// The ladder itself. Separate so it can be driven synchronously from a test.
|
||||
@@ -575,9 +593,24 @@ fn terminate_blocking(shared: &LeaseShared) {
|
||||
// gamescope owns the game: releasing the display tears the nested session — and therefore
|
||||
// the game — down. One mechanism for both, so a kept display can't outlive an ended game.
|
||||
LeaseKind::Nested => {
|
||||
// The same force-release the `/display/release` endpoint performs. It refuses displays
|
||||
// with live sessions, so this only ever retires the *kept* display this ended session
|
||||
// left behind — which is exactly the one still holding the game.
|
||||
// The same force-release the `/display/release` endpoint performs: gamescope exits with
|
||||
// its display and takes its nested game with it. It refuses displays that still have live
|
||||
// sessions, so this only reaches the *kept* display this ended session left behind.
|
||||
//
|
||||
// But it is not slot-targeted — it retires every kept display — and a lease has no handle
|
||||
// on which one is its own. So while anyone else is streaming, leave it alone: the cost of
|
||||
// being wrong here is disturbing an unrelated client, and the cost of doing nothing is a
|
||||
// game that keeps running, which is also the default everywhere else.
|
||||
let others = crate::session_status::count();
|
||||
if others > 0 {
|
||||
tracing::info!(
|
||||
live_sessions = others,
|
||||
title = %shared.game.title,
|
||||
"not releasing kept displays to end this game — another session is streaming and \
|
||||
the release is not per-display"
|
||||
);
|
||||
return;
|
||||
}
|
||||
let released = crate::vdisplay::registry::release(None);
|
||||
tracing::info!(
|
||||
released,
|
||||
|
||||
@@ -192,11 +192,16 @@ impl Scanner {
|
||||
}
|
||||
}
|
||||
if let Some(dir) = install_dir {
|
||||
// Require a path separator after the directory, so an install dir of `/games/x` is
|
||||
// not satisfied by an unrelated `/games/xyz/…` argument. (The image-path check above
|
||||
// needs no such care — `Path::starts_with` already compares whole components.)
|
||||
let needle = dir.as_os_str().as_encoded_bytes();
|
||||
for arg in cmdline.split(|&b| b == 0) {
|
||||
if arg.starts_with(needle) {
|
||||
return true;
|
||||
}
|
||||
let under_dir = |arg: &[u8]| {
|
||||
arg.strip_prefix(needle)
|
||||
.is_some_and(|rest| rest.first() == Some(&b'/'))
|
||||
};
|
||||
if cmdline.split(|&b| b == 0).any(under_dir) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
if let Some(want) = exe {
|
||||
@@ -376,6 +381,26 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_dir_does_not_match_a_sibling_with_the_same_prefix() {
|
||||
// `/games/x` must not adopt a process running out of `/games/xyz` — a real hazard when one
|
||||
// library folder's name is a prefix of another's.
|
||||
let td = fake_proc_root(
|
||||
1000.0,
|
||||
&[
|
||||
FakeProc::new(90, 50_000).exe("/games/xyz/other"),
|
||||
FakeProc::new(91, 50_000).cmdline(&["wrapper", "/games/xyz/other"]),
|
||||
],
|
||||
);
|
||||
let s = scanner(td.path());
|
||||
assert!(s.find(&DetectSpec::dir("/games/x"), None).is_empty());
|
||||
// The genuine directory still matches, by image path and by argument.
|
||||
assert_eq!(
|
||||
pids(s.find(&DetectSpec::dir("/games/xyz"), None)),
|
||||
vec![90, 91]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn matches_proton_style_game_only_in_the_cmdline() {
|
||||
// The image is the Proton runtime; the game appears only as an argument.
|
||||
|
||||
Reference in New Issue
Block a user