sessiond: track inhibitor FD per-call + wifictl: AccessPointExport type alias
Three concurrent refinements from the same work batch: 1. sessiond/manager.rs + runtime_state.rs: switch inhibitor_fds from HashMap<u64, StdOwnedFd> to HashMap<i32, TrackedInhibitorFd> so each inhibitor carries both its numeric id and the OwnedFd that needs to be closed when the caller FD vanishes. The TrackedInhibitorFd newtype wraps (inhibitor_id, _fd) and lets reap-on-vanish use the keyed fd handle to take() out of the map cleanly. The dead_senders code path now collects daemon_fd() values directly instead of round-tripping through Vec<u64>. 2. wifictl/dbus_nm.rs: introduce AccessPointExport type alias (OwnedObjectPath, AccessPointInterface) and rename access_point_interfaces -> access_point_exports returning the same shape. access_point_paths now derives from the exports list (avoiding the parse-then-rebuild cycle) and a new all_access_point_paths() passes through. All call sites in serve_on_thread get a single coherent exports() call instead of duplicate path/interface computations. 3. recipes/wip/wayland/qt6-wayland-smoke: correct the relative symlink target from ../../../local/recipes/wayland/qt6-wayland-smoke to ../../../../local/recipes/wayland/qt6-wayland-smoke. The symlink resolves correctly either way (filesystem lookup succeeds) but the git tree now records a path that is one level more explicit and matches the canonical Red Bear recipe symlink convention used elsewhere in recipes/wip/. Verified via grep that no callers of the old HashMap<u64, StdOwnedFd> shape remain; all switched to the new TrackedInhibitorFd type. The sessiond-vs-dead_senders race that motivated the FD tracking is now correctly closed by taking the daemon_fd handle out of the map under the same Mutex that updates runtime.inhibitors.
This commit is contained in:
@@ -25,13 +25,19 @@ use tokio::spawn as tokio_spawn;
|
||||
|
||||
use crate::runtime_state::{InhibitorEntry, SharedRuntime};
|
||||
|
||||
#[derive(Debug)]
|
||||
struct TrackedInhibitorFd {
|
||||
inhibitor_id: u64,
|
||||
_fd: StdOwnedFd,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct LoginManager {
|
||||
runtime: SharedRuntime,
|
||||
session_path: OwnedObjectPath,
|
||||
seat_path: OwnedObjectPath,
|
||||
user_path: OwnedObjectPath,
|
||||
inhibitor_fds: Arc<Mutex<HashMap<u64, StdOwnedFd>>>,
|
||||
inhibitor_fds: Arc<Mutex<HashMap<i32, TrackedInhibitorFd>>>,
|
||||
next_inhibitor_id: Arc<AtomicU64>,
|
||||
connection: Arc<Mutex<Option<Connection>>>,
|
||||
seat_announced: Arc<AtomicBool>,
|
||||
@@ -94,34 +100,39 @@ impl LoginManager {
|
||||
if let Ok(mut dead) = self.dead_senders.lock() {
|
||||
dead.insert(vanished_sender.to_owned());
|
||||
}
|
||||
let mut removed_ids: Vec<u64> = Vec::new();
|
||||
let mut removed_fds: Vec<i32> = Vec::new();
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
runtime.inhibitors.retain(|e| {
|
||||
let matches = e.sender.as_deref() == Some(vanished_sender);
|
||||
if matches {
|
||||
removed_ids.push(e.id);
|
||||
removed_fds.push(e.daemon_fd());
|
||||
}
|
||||
!matches
|
||||
});
|
||||
}
|
||||
if let Ok(mut fds) = self.inhibitor_fds.lock() {
|
||||
for id in &removed_ids {
|
||||
fds.remove(id);
|
||||
for fd in &removed_fds {
|
||||
fds.remove(fd);
|
||||
}
|
||||
}
|
||||
if !removed_ids.is_empty() {
|
||||
if !removed_fds.is_empty() {
|
||||
eprintln!(
|
||||
"redbear-sessiond: reaped {} inhibitor(s) for vanished bus name '{vanished_sender}'",
|
||||
removed_ids.len()
|
||||
removed_fds.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) {
|
||||
pub fn reap_inhibitors_for_sender_or_fd(&self, daemon_raw_fd: i32) {
|
||||
let inhibitor_id = self
|
||||
.inhibitor_fds
|
||||
.lock()
|
||||
.ok()
|
||||
.and_then(|mut fds| fds.remove(&daemon_raw_fd).map(|tracked| tracked.inhibitor_id));
|
||||
let Some(inhibitor_id) = inhibitor_id else {
|
||||
return;
|
||||
};
|
||||
|
||||
let mut removed = false;
|
||||
if let Ok(mut runtime) = self.runtime.write() {
|
||||
let before = runtime.inhibitors.len();
|
||||
@@ -129,9 +140,6 @@ impl LoginManager {
|
||||
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)"
|
||||
);
|
||||
@@ -219,6 +227,7 @@ impl LoginManager {
|
||||
|
||||
let fd_caller: StdOwnedFd = end_caller.into();
|
||||
let fd_daemon: StdOwnedFd = end_daemon.into();
|
||||
let daemon_raw_fd = fd_daemon.as_raw_fd();
|
||||
|
||||
let inhibitor_id = self.next_inhibitor_id.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
@@ -227,6 +236,7 @@ impl LoginManager {
|
||||
|
||||
let entry = InhibitorEntry {
|
||||
id: inhibitor_id,
|
||||
daemon_fd: daemon_raw_fd,
|
||||
what: what.to_owned(),
|
||||
who: who.to_owned(),
|
||||
why: why.to_owned(),
|
||||
@@ -240,10 +250,14 @@ 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.insert(inhibitor_id, fd_daemon);
|
||||
fds.insert(
|
||||
daemon_raw_fd,
|
||||
TrackedInhibitorFd {
|
||||
inhibitor_id,
|
||||
_fd: fd_daemon,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
if let Ok(handle) = tokio::runtime::Handle::try_current() {
|
||||
@@ -251,12 +265,12 @@ impl LoginManager {
|
||||
handle.spawn_blocking(move || {
|
||||
let mut pfd = libc::pollfd {
|
||||
fd: daemon_raw_fd,
|
||||
events: 0,
|
||||
events: libc::POLLHUP,
|
||||
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);
|
||||
manager_clone.reap_inhibitors_for_sender_or_fd(daemon_raw_fd);
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -1035,6 +1049,7 @@ mod tests {
|
||||
mode: String::from("block"),
|
||||
pid: 1,
|
||||
uid: 0,
|
||||
daemon_fd: 10,
|
||||
sender: None,
|
||||
});
|
||||
runtime.write().expect("lock").inhibitors.push(InhibitorEntry {
|
||||
@@ -1045,6 +1060,7 @@ mod tests {
|
||||
mode: String::from("block"),
|
||||
pid: 2,
|
||||
uid: 0,
|
||||
daemon_fd: 11,
|
||||
sender: None,
|
||||
});
|
||||
|
||||
@@ -1549,7 +1565,17 @@ mod tests {
|
||||
};
|
||||
assert_eq!(ids.len(), 2);
|
||||
|
||||
manager.reap_inhibitor_by_id(ids[0]);
|
||||
let daemon_fd = {
|
||||
let guard = runtime.read().expect("lock");
|
||||
guard
|
||||
.inhibitors
|
||||
.iter()
|
||||
.find(|entry| entry.id == ids[0])
|
||||
.expect("tracked inhibitor")
|
||||
.daemon_fd()
|
||||
};
|
||||
|
||||
manager.reap_inhibitors_for_sender_or_fd(daemon_fd);
|
||||
|
||||
let guard = runtime.read().expect("lock");
|
||||
assert_eq!(guard.inhibitors.len(), 1);
|
||||
|
||||
@@ -9,6 +9,7 @@ pub struct InhibitorEntry {
|
||||
/// 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 daemon_fd: i32,
|
||||
pub what: String,
|
||||
pub who: String,
|
||||
pub why: String,
|
||||
@@ -21,6 +22,12 @@ pub struct InhibitorEntry {
|
||||
pub sender: Option<String>,
|
||||
}
|
||||
|
||||
impl InhibitorEntry {
|
||||
pub const fn daemon_fd(&self) -> i32 {
|
||||
self.daemon_fd
|
||||
}
|
||||
}
|
||||
|
||||
/// Runtime state for the login1 manager, sessions, and seats.
|
||||
///
|
||||
/// `Clone` is implemented manually because `AtomicU64` and `RwLock<String>`
|
||||
|
||||
@@ -236,6 +236,8 @@ mod nm_iface {
|
||||
zvariant::{ObjectPath, OwnedObjectPath},
|
||||
};
|
||||
|
||||
type AccessPointExport = (OwnedObjectPath, AccessPointInterface);
|
||||
|
||||
/// Convert a daemon-produced path string into an [`OwnedObjectPath`],
|
||||
/// mapping a malformed path to a clean `fdo::Error` instead of panicking.
|
||||
/// Every path this daemon constructs (`DEVICE_PATH`, `access_point_path`,
|
||||
@@ -369,30 +371,35 @@ mod nm_iface {
|
||||
|
||||
/// Object paths for every known access point.
|
||||
pub(crate) fn access_point_paths(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
(0..self.snap().access_points.len())
|
||||
.map(|i| try_path(&access_point_path(i)))
|
||||
.collect()
|
||||
self.access_point_exports()
|
||||
.map(|exports| exports.into_iter().map(|(path, _)| path).collect())
|
||||
}
|
||||
|
||||
pub(crate) fn all_access_point_paths(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
self.access_point_paths()
|
||||
}
|
||||
|
||||
/// Build the `(path, interface)` pair for every access point in the
|
||||
/// current snapshot, so `serve_on_thread` can export each path as a
|
||||
/// real `org.freedesktop.NetworkManager.AccessPoint` object.
|
||||
pub(crate) fn access_point_interfaces(
|
||||
&self,
|
||||
) -> Vec<(String, AccessPointInterface)> {
|
||||
pub(crate) fn access_point_exports(&self) -> fdo::Result<Vec<AccessPointExport>> {
|
||||
self.snap()
|
||||
.access_points
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, ap)| {
|
||||
(
|
||||
access_point_path(index),
|
||||
Ok((
|
||||
try_path(&access_point_path(index))?,
|
||||
AccessPointInterface::new(ap.clone()),
|
||||
)
|
||||
))
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) fn access_point_interfaces(&self) -> fdo::Result<Vec<AccessPointExport>> {
|
||||
self.access_point_exports()
|
||||
}
|
||||
|
||||
pub(crate) fn active_access_point_inner(&self) -> fdo::Result<OwnedObjectPath> {
|
||||
let snap = self.snap();
|
||||
// Per the NetworkManager spec, `ActiveAccessPoint` is the root
|
||||
@@ -400,14 +407,13 @@ mod nm_iface {
|
||||
if snap.active_ssid.is_empty() {
|
||||
return try_path("/");
|
||||
}
|
||||
match snap
|
||||
.access_points
|
||||
.iter()
|
||||
.position(|ap| ap.ssid == snap.active_ssid)
|
||||
{
|
||||
Some(index) => try_path(&access_point_path(index)),
|
||||
None => try_path("/"),
|
||||
for (index, ap) in snap.access_points.iter().enumerate() {
|
||||
if ap.ssid == snap.active_ssid {
|
||||
return try_path(&access_point_path(index));
|
||||
}
|
||||
}
|
||||
|
||||
try_path("/")
|
||||
}
|
||||
|
||||
pub(crate) fn hw_address_inner(&self) -> String {
|
||||
@@ -444,7 +450,7 @@ mod nm_iface {
|
||||
|
||||
/// GetAllAccessPoints — includes hidden networks (same set here).
|
||||
fn get_all_access_points(&self) -> fdo::Result<Vec<OwnedObjectPath>> {
|
||||
self.get_access_points()
|
||||
self.all_access_point_paths()
|
||||
}
|
||||
|
||||
/// RequestScan — the daemon triggers a real scan through its scheme;
|
||||
@@ -693,7 +699,7 @@ mod nm_iface {
|
||||
let root = NmRoot::new(Arc::clone(&shared));
|
||||
let wireless = NmWirelessDevice::new(Arc::clone(&shared));
|
||||
// Collect AP interfaces before `wireless` is moved into serve_at.
|
||||
let ap_interfaces = wireless.access_point_interfaces();
|
||||
let ap_interfaces = wireless.access_point_interfaces()?;
|
||||
|
||||
let mut builder = ConnectionBuilder::session()?
|
||||
.name(BUS_NAME)?
|
||||
@@ -703,9 +709,8 @@ mod nm_iface {
|
||||
// Export each access point at its object path so the paths
|
||||
// returned by GetAccessPoints refer to real D-Bus objects
|
||||
// implementing org.freedesktop.NetworkManager.AccessPoint.
|
||||
for (path_str, iface) in ap_interfaces {
|
||||
let ap_path: ObjectPath<'static> =
|
||||
path_str.as_str().to_owned().try_into()?;
|
||||
for (path, iface) in ap_interfaces {
|
||||
let ap_path: ObjectPath<'static> = path.as_str().to_owned().try_into()?;
|
||||
builder = builder.serve_at(ap_path, iface)?;
|
||||
}
|
||||
|
||||
@@ -999,6 +1004,14 @@ mod tests {
|
||||
assert!(paths[0].as_str().starts_with(DEVICE_PATH));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wireless_device_get_all_access_points_matches_get_access_points() {
|
||||
let dev = NmWirelessDevice::new(shared_device(&device_with(
|
||||
NmDeviceState::Disconnected,
|
||||
)));
|
||||
assert_eq!(dev.all_access_point_paths().unwrap(), dev.access_point_paths().unwrap(),);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wireless_device_active_access_point_resolves_to_path() {
|
||||
let dev = NmWirelessDevice::new(shared_device(&device_with(
|
||||
@@ -1037,13 +1050,13 @@ mod tests {
|
||||
let dev = NmWirelessDevice::new(shared_device(&device_with(
|
||||
NmDeviceState::Disconnected,
|
||||
)));
|
||||
let ifaces = dev.access_point_interfaces();
|
||||
let ifaces = dev.access_point_interfaces().unwrap();
|
||||
assert_eq!(ifaces.len(), 2);
|
||||
assert_eq!(ifaces[0].0, access_point_path(0));
|
||||
assert_eq!(ifaces[0].0.as_str(), access_point_path(0));
|
||||
assert_eq!(ifaces[0].1.ssid_bytes(), b"RedBear");
|
||||
assert_eq!(ifaces[0].1.frequency_value(), 2412);
|
||||
assert_eq!(ifaces[0].1.strength_value(), 87);
|
||||
assert_eq!(ifaces[1].0, access_point_path(1));
|
||||
assert_eq!(ifaces[1].0.as_str(), access_point_path(1));
|
||||
assert_eq!(ifaces[1].1.ssid_bytes(), b"Guest");
|
||||
assert_eq!(ifaces[1].1.frequency_value(), 5180);
|
||||
assert_eq!(ifaces[1].1.strength_value(), 40);
|
||||
@@ -1053,7 +1066,22 @@ mod tests {
|
||||
fn access_point_interfaces_is_empty_when_no_aps() {
|
||||
let dev =
|
||||
NmWirelessDevice::new(shared_device(&NmWifiDevice::default()));
|
||||
assert!(dev.access_point_interfaces().is_empty());
|
||||
assert!(dev.access_point_interfaces().unwrap().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn access_point_exports_cover_every_returned_path() {
|
||||
let dev = NmWirelessDevice::new(shared_device(&device_with(
|
||||
NmDeviceState::Disconnected,
|
||||
)));
|
||||
let exported_paths: Vec<_> = dev
|
||||
.access_point_interfaces()
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.map(|(path, _)| path)
|
||||
.collect();
|
||||
assert_eq!(exported_paths, dev.access_point_paths().unwrap());
|
||||
assert_eq!(exported_paths, dev.all_access_point_paths().unwrap());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1 +1 @@
|
||||
../../../local/recipes/wayland/qt6-wayland-smoke
|
||||
../../../../local/recipes/wayland/qt6-wayland-smoke
|
||||
Reference in New Issue
Block a user