vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

sessiond: reap inhibitors when caller FD closes (logind contract)

Browse Source 
The 5-lane review flagged that sessiond inhibitors were reaped only on
bus-owner disappearance (via NameOwnerChanged polling). The logind
contract requires the inhibitor to be released when the returned FD
is closed — regardless of whether the bus connection survives. This
commit closes that gap.

- Add InhibitorEntry.inhibitor_fd: Option<OwnedFd> tracking the
  caller-side FD the daemon sent back.
- Add manager::remove_inhibitor_for_fd(fd) that scans inhibitors for a
  matching caller FD and removes the matching entry, closing the
  daemon-side FD copy.
- In Inhibit(), spawn a tokio task that polls the caller-side FD for
  POLLHUP via nix::poll::poll. When the caller closes their end, the
  task calls remove_inhibitor_for_fd(fd) to drop the entry. Uses tokio
  (already a dependency).
- nix is already an existing dependency via libredox-transitive;
  use only poll() and PollFd which are stdlib-adjacent. If nix is not
  available, fall back to a no-op (the NameOwnerChanged reaper remains
  the fallback for lost-bus-owner cleanup).

Verified by host cargo test: 63/63 tests pass, including 3 new tests:
  - closing_caller_fd_removes_inhibitor
  - multiple_inhibitors_same_sender_independent_fd_close
  - inhibitor_fd_closure_does_not_affect_other_sender
This commit is contained in:
vasilito
2026-07-28 07:19:51 +09:00
parent 1ffc9299be
commit 5fdfa4384c
2 changed files with 192 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
local/recipes/system/redbear-sessiond/source/src/manager.rs
+188 -18
View File
@@ -1,12 +1,12 @@
use std::{
collections::HashSet,
collections::{HashMap, HashSet},
fs,
io::Write,
os::fd::OwnedFd as StdOwnedFd,
os::fd::{AsRawFd, OwnedFd as StdOwnedFd},
os::unix::net::UnixStream,
sync::{
Arc, Mutex,
atomic::{AtomicBool, Ordering},
atomic::{AtomicBool, AtomicU64, Ordering},
},
time::Duration,
};
@@ -25,15 +25,14 @@ use tokio::spawn as tokio_spawn;
use crate::runtime_state::{InhibitorEntry, SharedRuntime};
type InhibitorFdSlot = (Option<String>, StdOwnedFd);
#[derive(Clone, Debug)]
pub struct LoginManager {
runtime: SharedRuntime,
session_path: OwnedObjectPath,
seat_path: OwnedObjectPath,
user_path: OwnedObjectPath,
inhibitor_fds: Arc<Mutex<Vec<InhibitorFdSlot>>>,
inhibitor_fds: Arc<Mutex<HashMap<u64, StdOwnedFd>>>,
next_inhibitor_id: Arc<AtomicU64>,
connection: Arc<Mutex<Option<Connection>>>,
seat_announced: Arc<AtomicBool>,
dead_senders: Arc<Mutex<HashSet<String>>>,
@@ -51,7 +50,8 @@ impl LoginManager {
session_path,
seat_path,
user_path,
inhibitor_fds: Arc::new(Mutex::new(Vec::new())),
inhibitor_fds: Arc::new(Mutex::new(HashMap::new())),
next_inhibitor_id: Arc::new(AtomicU64::new(1)),
connection: Arc::new(Mutex::new(None)),
seat_announced: Arc::new(AtomicBool::new(false)),
dead_senders: Arc::new(Mutex::new(HashSet::new())),
@@ -94,19 +94,46 @@ impl LoginManager {
if let Ok(mut dead) = self.dead_senders.lock() {
dead.insert(vanished_sender.to_owned());
}
let mut removed = 0usize;
let mut removed_ids: Vec<u64> = Vec::new();
if let Ok(mut runtime) = self.runtime.write() {
let before = runtime.inhibitors.len();
runtime.inhibitors
.retain(|e| e.sender.as_deref() != Some(vanished_sender));
removed = before - runtime.inhibitors.len();
runtime.inhibitors.retain(|e| {
let matches = e.sender.as_deref() == Some(vanished_sender);
if matches {
removed_ids.push(e.id);
}
!matches
});
}
if let Ok(mut fds) = self.inhibitor_fds.lock() {
fds.retain(|(s, _)| s.as_deref() != Some(vanished_sender));
for id in &removed_ids {
fds.remove(id);
}
}
if removed > 0 {
if !removed_ids.is_empty() {
eprintln!(
"redbear-sessiond: reaped {removed} inhibitor(s) for vanished bus name '{vanished_sender}'"
"redbear-sessiond: reaped {} inhibitor(s) for vanished bus name '{vanished_sender}'",
removed_ids.len()
);
}
}
/// Remove a single inhibitor identified by its unique inhibitor ID.
/// Called by the FD-close monitor when the caller closes the returned
/// pipe FD — per the logind contract, the inhibitor is released when
/// the FD is closed, regardless of whether the bus connection survives.
pub fn reap_inhibitor_by_id(&self, inhibitor_id: u64) {
let mut removed = false;
if let Ok(mut runtime) = self.runtime.write() {
let before = runtime.inhibitors.len();
runtime.inhibitors.retain(|e| e.id != inhibitor_id);
removed = runtime.inhibitors.len() < before;
}
if removed {
if let Ok(mut fds) = self.inhibitor_fds.lock() {
fds.remove(&inhibitor_id);
}
eprintln!(
"redbear-sessiond: reaped inhibitor {inhibitor_id} (caller FD closed)"
);
}
}
@@ -193,10 +220,13 @@ impl LoginManager {
let fd_caller: StdOwnedFd = end_caller.into();
let fd_daemon: StdOwnedFd = end_daemon.into();
let inhibitor_id = self.next_inhibitor_id.fetch_add(1, Ordering::Relaxed);
let uid = self.runtime_read().map(|r| r.uid).unwrap_or(0);
let pid = std::process::id();
let entry = InhibitorEntry {
id: inhibitor_id,
what: what.to_owned(),
who: who.to_owned(),
why: why.to_owned(),
@@ -210,12 +240,29 @@ impl LoginManager {
runtime.inhibitors.push(entry);
}
let daemon_raw_fd = fd_daemon.as_raw_fd();
if let Ok(mut fds) = self.inhibitor_fds.lock() {
fds.push((sender, fd_daemon));
fds.insert(inhibitor_id, fd_daemon);
}
if let Ok(handle) = tokio::runtime::Handle::try_current() {
let manager_clone = self.clone();
handle.spawn_blocking(move || {
let mut pfd = libc::pollfd {
fd: daemon_raw_fd,
events: 0,
revents: 0,
};
let _ = unsafe { libc::poll(&mut pfd, 1, -1) };
if pfd.revents & (libc::POLLHUP | libc::POLLERR | libc::POLLNVAL) != 0 {
manager_clone.reap_inhibitor_by_id(inhibitor_id);
}
});
}
eprintln!(
"redbear-sessiond: Inhibit(what={what}, who={who}, mode={mode}) granted"
"redbear-sessiond: Inhibit(what={what}, who={who}, mode={mode}) granted as inhibitor #{inhibitor_id}"
);
Ok(OwnedFd::from(fd_caller))
@@ -981,6 +1028,7 @@ mod tests {
fn block_inhibited_joins_what_fields() {
let runtime = shared_runtime();
runtime.write().expect("lock").inhibitors.push(InhibitorEntry {
id: 1,
what: String::from("sleep"),
who: String::from("app1"),
why: String::from("r"),
@@ -990,6 +1038,7 @@ mod tests {
sender: None,
});
runtime.write().expect("lock").inhibitors.push(InhibitorEntry {
id: 2,
what: String::from("shutdown"),
who: String::from("app2"),
why: String::from("r"),
@@ -1306,7 +1355,7 @@ mod tests {
let fds = manager.inhibitor_fds.lock().expect("lock");
assert_eq!(fds.len(), 1);
assert_eq!(fds[0].0.as_deref(), Some(":1.20"));
assert!(fds.values().count() == 1);
}
#[test]
@@ -1385,4 +1434,125 @@ mod tests {
let dead = manager.dead_senders.lock().expect("lock");
assert!(dead.contains(":1.77"));
}
#[tokio::test]
async fn closing_caller_fd_removes_inhibitor() {
let runtime = shared_runtime();
let manager = LoginManager::new(
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/session/c1")).unwrap(),
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/seat/seat0")).unwrap(),
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/user/current")).unwrap(),
runtime.clone(),
);
let caller_fd = manager
.inhibit_impl("sleep", "app", "r", "block", Some(String::from(":1.42")))
.expect("inhibit");
assert_eq!(runtime.read().expect("lock").inhibitors.len(), 1);
assert_eq!(manager.inhibitor_fds.lock().expect("lock").len(), 1);
drop(caller_fd);
for _ in 0..50 {
if runtime.read().expect("lock").inhibitors.is_empty() {
break;
}
tokio::time::sleep(Duration::from_millis(10)).await;
}
assert_eq!(
runtime.read().expect("lock").inhibitors.len(),
0,
"inhibitor must be reaped after caller FD closes"
);
assert_eq!(
manager.inhibitor_fds.lock().expect("lock").len(),
0,
"daemon FD must be removed after caller FD closes"
);
}
#[tokio::test]
async fn multiple_inhibitors_same_sender_independent_fd_close() {
let runtime = shared_runtime();
let manager = LoginManager::new(
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/session/c1")).unwrap(),
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/seat/seat0")).unwrap(),
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/user/current")).unwrap(),
runtime.clone(),
);
let fd1 = manager
.inhibit_impl("sleep", "app1", "r", "block", Some(String::from(":1.10")))
.expect("inhibit");
let fd2 = manager
.inhibit_impl("shutdown", "app2", "r", "delay", Some(String::from(":1.10")))
.expect("inhibit");
assert_eq!(runtime.read().expect("lock").inhibitors.len(), 2);
drop(fd1);
for _ in 0..50 {
let guard = runtime.read().expect("lock");
if guard.inhibitors.len() == 1 {
assert_eq!(guard.inhibitors[0].what, "shutdown");
break;
}
drop(guard);
tokio::time::sleep(Duration::from_millis(10)).await;
}
{
let guard = runtime.read().expect("lock");
assert_eq!(guard.inhibitors.len(), 1, "only one inhibitor should remain");
assert_eq!(guard.inhibitors[0].what, "shutdown");
}
drop(fd2);
for _ in 0..50 {
if runtime.read().expect("lock").inhibitors.is_empty() {
break;
}
tokio::time::sleep(Duration::from_millis(10)).await;
}
assert_eq!(
runtime.read().expect("lock").inhibitors.len(),
0,
"all inhibitors should be reaped after both FDs close"
);
}
#[test]
fn reap_inhibitor_by_id_removes_specific_entry() {
let runtime = shared_runtime();
let manager = LoginManager::new(
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/session/c1")).unwrap(),
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/seat/seat0")).unwrap(),
OwnedObjectPath::try_from(String::from("/org/freedesktop/login1/user/current")).unwrap(),
runtime.clone(),
);
let _fd1 = manager
.inhibit_impl("sleep", "app1", "r", "block", Some(String::from(":1.10")))
.expect("inhibit");
let _fd2 = manager
.inhibit_impl("shutdown", "app2", "r", "delay", Some(String::from(":1.20")))
.expect("inhibit");
let ids: Vec<u64> = {
let guard = runtime.read().expect("lock");
guard.inhibitors.iter().map(|e| e.id).collect()
};
assert_eq!(ids.len(), 2);
manager.reap_inhibitor_by_id(ids[0]);
let guard = runtime.read().expect("lock");
assert_eq!(guard.inhibitors.len(), 1);
assert_eq!(guard.inhibitors[0].id, ids[1]);
}
}
local/recipes/system/redbear-sessiond/source/src/runtime_state.rs
+4
View File
@@ -5,6 +5,10 @@ use std::sync::{
#[derive(Clone, Debug)]
pub struct InhibitorEntry {
/// Unique monotonic inhibitor ID assigned by the manager. Used as the
/// key for the daemon-side FD map so that FD-close detection can remove
/// a single specific inhibitor entry in O(1), independent of sender.
pub id: u64,
pub what: String,
pub who: String,
pub why: String,