diff --git a/compositor/src/backends/direct.rs b/compositor/src/backends/direct.rs index 2662eee..42b6f4e 100644 --- a/compositor/src/backends/direct.rs +++ b/compositor/src/backends/direct.rs @@ -17,6 +17,7 @@ mod planes; mod recovery; mod redraw; mod scheduling; +mod session_device; mod topology; mod transaction; use diagnostics::{Diagnostics, PrimaryOutcome}; @@ -47,10 +48,9 @@ use smithay::reexports::drm::control::{ Device as ControlDevice, Mode as DrmMode, ModeTypeFlags, connector, crtc, property, }; use smithay::reexports::input::{Device as LibinputDevice, Libinput}; -use smithay::reexports::rustix::fs::OFlags; use smithay::reexports::wayland_protocols::wp::presentation_time::server::wp_presentation_feedback::Kind; use smithay::reexports::wayland_server::backend::GlobalId; -use smithay::utils::{DeviceFd, Monotonic, Time, Transform}; +use smithay::utils::{Monotonic, Time, Transform}; use smithay::wayland::dmabuf::DmabufFeedbackBuilder; use smithay::wayland::presentation::Refresh; use topology::{Topology, reconcile_outputs}; @@ -257,6 +257,8 @@ struct DirectDevice { renderer: GlesRenderer, render_node: DrmNode, outputs: HashMap, + // Last: release libseat's original FD after renderer/DRM resources. + _session_device: session_device::SessionDevice, } struct DirectOutput { @@ -788,15 +790,15 @@ pub(crate) fn validate_live_outputs(state: &Ferese, profiles: &[OutputProfile]) } fn open_device(state: &mut Ferese, path: &Path) -> Result> { - let mut session = state + let session = state .direct_backend .as_ref() .expect("direct backend state is initialized before the DRM device") .session .clone(); let node = DrmNode::from_path(path)?; - let fd = session.open(path, OFlags::RDWR | OFlags::CLOEXEC | OFlags::NOCTTY | OFlags::NONBLOCK)?; - let fd = DrmDeviceFd::new(DeviceFd::from(fd)); + let session_device = session_device::SessionDevice::open(session, path)?; + let fd = DrmDeviceFd::new(session_device.device_fd()?); let (drm, notifier) = DrmDevice::new(fd.clone(), true)?; let gbm = GbmDevice::new(fd)?; // SAFETY: GBM owns a valid DRM descriptor for the lifetime of the EGL display. @@ -984,6 +986,7 @@ fn open_device(state: &mut Ferese, path: &Path) -> Result { + session: S, + fd: Option, +} + +impl SessionDevice { + pub fn open(mut session: S, path: &Path) -> Result { + let fd = session.open(path, OFlags::RDWR | OFlags::CLOEXEC | OFlags::NOCTTY | OFlags::NONBLOCK)?; + Ok(Self { session, fd: Some(fd) }) + } + + pub fn device_fd(&self) -> io::Result { + self.fd.as_ref().unwrap().try_clone().map(DeviceFd::from) + } +} + +impl Drop for SessionDevice { + fn drop(&mut self) { + if let Err(error) = self.session.close(self.fd.take().unwrap()) { + tracing::warn!(?error, "could not release DRM device through the session manager"); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::{cell::Cell, fs::File, os::fd::AsRawFd, rc::Rc}; + + #[derive(Clone, Default)] + struct TestSession(Rc>>); + + impl Session for TestSession { + type Error = (); + fn open(&mut self, _: &Path, _: OFlags) -> Result { + if self.0.get().is_some() { + return Err(()); + } + let fd: OwnedFd = File::open("/dev/null").unwrap().into(); + self.0.set(Some(fd.as_raw_fd())); + Ok(fd) + } + fn close(&mut self, fd: OwnedFd) -> Result<(), ()> { + assert_eq!(self.0.replace(None), Some(fd.as_raw_fd())); + Ok(()) + } + fn change_vt(&mut self, _: i32) -> Result<(), ()> { + Ok(()) + } + fn is_active(&self) -> bool { + true + } + fn seat(&self) -> String { + "seat0".into() + } + } + + #[test] + fn teardown_releases_original_session_lease_before_reopening() { + let session = TestSession::default(); + for _ in 0..3 { + let lease = SessionDevice::open(session.clone(), Path::new("/dev/null")).unwrap(); + let rendering = lease.device_fd().unwrap(); + assert_ne!(Some(rendering.as_raw_fd()), session.0.get()); + drop(rendering); + assert!(session.0.get().is_some()); + drop(lease); + assert!(session.0.get().is_none()); + } + } + + #[test] + fn failed_renderer_initialization_releases_lease() { + fn initialize(session: TestSession) -> Result<(), ()> { + let lease = SessionDevice::open(session, Path::new("/dev/null"))?; + let _rendering = lease.device_fd().unwrap(); + Err(()) + } + let session = TestSession::default(); + assert!(initialize(session.clone()).is_err()); + assert!(session.0.get().is_none()); + assert!(SessionDevice::open(session, Path::new("/dev/null")).is_ok()); + } +} diff --git a/compositor/src/backends/direct/topology.rs b/compositor/src/backends/direct/topology.rs index 2f2acbe..6ec943e 100644 --- a/compositor/src/backends/direct/topology.rs +++ b/compositor/src/backends/direct/topology.rs @@ -260,7 +260,7 @@ pub(super) fn reconcile_outputs(state: &mut Ferese, reactivate: bool) { None => match open_device(state, current_path.unwrap()) { Ok(node) => node, Err(error) => { - tracing::warn!(%error, ?path, "DRM device unavailable; will retry"); + tracing::warn!(%error, ?path, device_path = ?current_path, "DRM device unavailable; will retry"); slot_mut(state, &path).failed(Instant::now()); continue; } diff --git a/shell/ferese-shell/src/panel_hub.rs b/shell/ferese-shell/src/panel_hub.rs index b433e00..1520073 100644 --- a/shell/ferese-shell/src/panel_hub.rs +++ b/shell/ferese-shell/src/panel_hub.rs @@ -456,7 +456,10 @@ impl FereseShell { } pub(super) fn view_hub(&self, id: window::Id) -> Element<'_, cosmic::Action> { - let entry = self.outputs.iter().find(|entry| entry.bar == id).expect("panel output"); + // The toolkit may request one last frame while output destruction is queued. + let Some(entry) = self.outputs.iter().find(|entry| entry.bar == id) else { + return cosmic::widget::Space::new().into(); + }; let runtime = entry.runtime.hub.as_ref().expect("Morph runtime"); runtime.borrow_mut().motion = self.render.motion; let revision = runtime.borrow().revision; @@ -542,8 +545,10 @@ impl FereseShell { menu: Option, previous_menu: Option, ) -> Element<'_, cosmic::Action> { + let Some(entry) = self.outputs.iter().find(|entry| entry.bar == id) else { + return cosmic::widget::Space::new().into(); + }; let panel = self.panel_for_bar(id); - let entry = self.outputs.iter().find(|entry| entry.bar == id).expect("panel output"); let effects = entry.effects.as_ref(); let bounds = sample.geometry.bounds; let (target_size, previous_size) = { @@ -1275,6 +1280,16 @@ mod tests { shell } + #[test] + fn queued_hub_frame_after_output_removal_is_empty_instead_of_panicking() { + let mut app = shell(); + let id = app.outputs[0].bar; + let sample = app.outputs[0].runtime.hub.as_ref().unwrap().borrow().hub.sample(); + app.outputs.clear(); + drop(app.view_hub(id)); + drop(app.view_hub_at(id, sample, None, None)); + } + #[test] fn saved_regular_margins_do_not_constrain_morph() { let mut app = shell();