Release session devices during teardown and tolerate removed panel outputs

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<crtc::Handle, DirectOutput>,
+    // Last: release libseat's original FD after renderer/DRM resources.
+    _session_device: session_device::SessionDevice<LibSeatSession>,
 }
 
 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<DrmNode, Box<dyn Error>> {
-    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 session_device = session_device::SessionDevice::open(session, path)?;
-    let fd = DrmDeviceFd::new(DeviceFd::from(fd));
+    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<DrmNode, Box<dyn Error
         .insert(
             node,
             DirectDevice {
+                _session_device: session_device,
                 drm,
                 gbm,
                 renderer,

compositor/src/backends/direct/session_device.rs

@@ -0,0 +1,87 @@
+use smithay::{backend::session::Session, reexports::rustix::fs::OFlags, utils::DeviceFd};
+use std::{io, os::fd::OwnedFd, path::Path};
+
+pub(super) struct SessionDevice<S: Session> {
+    session: S,
+    fd: Option<OwnedFd>,
+}
+
+impl<S: Session> SessionDevice<S> {
+    pub fn open(mut session: S, path: &Path) -> Result<Self, S::Error> {
+        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<DeviceFd> {
+        self.fd.as_ref().unwrap().try_clone().map(DeviceFd::from)
+    }
+}
+
+impl<S: Session> Drop for SessionDevice<S> {
+    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<Cell<Option<i32>>>);
+
+    impl Session for TestSession {
+        type Error = ();
+        fn open(&mut self, _: &Path, _: OFlags) -> Result<OwnedFd, ()> {
+            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());
+    }
+}

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;
                 }

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<Message>> {
-        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<status_ui::Menu>,
         previous_menu: Option<status_ui::Menu>,
     ) -> Element<'_, cosmic::Action<Message>> {
+        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();