Refactor udev backend

- Do not open devices for `UdevHandler` anymore
- `UdevBackend` does not require `LoopHandle` or `Session` anymore
- Type of the created device can be choosed freely by the handler
- `UdevBackendObserver` is not needed anymore

.travis.yml

@@ -39,7 +39,6 @@ env:
     - FEATURES="backend_libinput"
     - FEATURES="backend_udev"
     - FEATURES="backend_session"
-    - FEATURES="backend_session_udev"
     - FEATURES="backend_session_logind"
     - FEATURES="renderer_glium"
     - FEATURES="xwayland"

Cargo.toml

@@ -43,9 +43,8 @@ backend_drm = ["drm", "backend_egl"]
 backend_egl = ["gl_generator"]
 backend_libinput = ["input"]
 backend_session = []
-backend_session_udev = ["udev", "backend_session"]
+backend_udev = ["udev"]
 backend_session_logind = ["dbus", "systemd", "backend_session"]
-backend_udev = ["udev", "backend_drm", "backend_session_udev"]
 renderer_gl = ["gl_generator"]
 renderer_glium = ["renderer_gl", "glium"]
 xwayland = []

src/backend/udev.rs

@@ -9,258 +9,91 @@
 //! See also `examples/udev.rs` for pure hardware backed example of a compositor utilizing this
 //! backend.
 
-use backend::{
+use nix::sys::stat::{dev_t, stat};
-    drm::{drm_device_bind, DrmDevice, DrmHandler},
-    session::{AsSessionObserver, Session, SessionObserver},
-};
-use drm::{control::Device as ControlDevice, Device as BasicDevice};
-use nix::{fcntl, sys::stat::dev_t};
 use std::{
-    cell::RefCell,
+    collections::HashSet,
-    collections::HashMap,
     ffi::OsString,
-    io::Error as IoError,
-    mem::drop,
     os::unix::io::{AsRawFd, RawFd},
     path::{Path, PathBuf},
-    rc::{Rc, Weak},
 };
-use udev::{Context, Enumerator, Event, EventType, MonitorBuilder, MonitorSocket, Result as UdevResult};
+use udev::{Context, Enumerator, EventType, MonitorBuilder, MonitorSocket, Result as UdevResult};
 
 use wayland_server::calloop::{
-    generic::{EventedRawFd, Generic},
+    generic::{EventedFd, Generic},
     mio::Ready,
-    LoopHandle, Source,
+    LoopHandle, Source, InsertError,
 };
 
-/// Udev's `DrmDevice` type based on the underlying session
+/// Backend to monitor available drm devices.
-pub struct SessionFdDrmDevice(RawFd);
-
-impl AsRawFd for SessionFdDrmDevice {
-    fn as_raw_fd(&self) -> RawFd {
-        self.0
-    }
-}
-impl BasicDevice for SessionFdDrmDevice {}
-impl ControlDevice for SessionFdDrmDevice {}
-
-/// Graphical backend that monitors available DRM devices.
 ///
-/// Provides a way to automatically initialize a `DrmDevice` for available GPUs and notifies the
+/// Provides a way to automatically scan for available gpus and notifies the
-/// given handler of any changes. Can be used to provide hot-plug functionality for GPUs and
+/// given handler of any changes. Can be used to provide hot-plug functionality for gpus and
 /// attached monitors.
-pub struct UdevBackend<
+pub struct UdevBackend<T: UdevHandler + 'static> {
-    H: DrmHandler<SessionFdDrmDevice> + 'static,
+    devices: HashSet<dev_t>,
-    S: Session + 'static,
-    T: UdevHandler<H> + 'static,
-    Data: 'static,
-> {
-    _handler: ::std::marker::PhantomData<H>,
-    devices: Rc<
-        RefCell<
-            HashMap<
-                dev_t,
-                (
-                    Source<Generic<EventedRawFd>>,
-                    Rc<RefCell<DrmDevice<SessionFdDrmDevice>>>,
-                ),
-            >,
-        >,
-    >,
     monitor: MonitorSocket,
-    session: S,
     handler: T,
     logger: ::slog::Logger,
-    handle: LoopHandle<Data>,
 }
 
-impl<
+impl<T: UdevHandler + 'static> AsRawFd for UdevBackend<T> {
-        H: DrmHandler<SessionFdDrmDevice> + 'static,
+    fn as_raw_fd(&self) -> RawFd {
-        S: Session + 'static,
+        self.monitor.as_raw_fd()
-        T: UdevHandler<H> + 'static,
+    }
-        Data: 'static,
+}
-    > UdevBackend<H, S, T, Data>
+
-{
+impl<T: UdevHandler + 'static> UdevBackend<T> {
     /// Creates a new `UdevBackend` and adds it to the given `EventLoop`'s state.
     ///
     /// ## Arguments
-    /// `evlh` - An event loop to use for binding `DrmDevices`
     /// `context` - An initialized udev context
-    /// `session` - A session used to open and close devices as they become available
     /// `handler` - User-provided handler to respond to any detected changes
+    /// `seat`    - 
     /// `logger`  - slog Logger to be used by the backend and its `DrmDevices`.
-    pub fn new<L>(
+    pub fn new<L, S: AsRef<str>>(
-        handle: LoopHandle<Data>,
         context: &Context,
-        mut session: S,
         mut handler: T,
+        seat: S,
         logger: L,
-    ) -> Result<UdevBackend<H, S, T, Data>>
+    ) -> UdevResult<UdevBackend<T>>
     where
         L: Into<Option<::slog::Logger>>,
     {
-        let logger = ::slog_or_stdlog(logger).new(o!("smithay_module" => "backend_udev"));
+        let log = ::slog_or_stdlog(logger).new(o!("smithay_module" => "backend_udev"));
-        let seat = session.seat();
+
-        let devices = all_gpus(context, seat)
+        let devices = all_gpus(context, seat)?
-            .chain_err(|| ErrorKind::FailedToScan)?
             .into_iter()
             // Create devices
-            .flat_map(|path| {
+            .flat_map(|path| match stat(&path) {
-                match DrmDevice::new(
+                Ok(stat) => {
-                    {
+                    handler.device_added(stat.st_rdev, path);  
-                        match session.open(
+                    Some(stat.st_rdev)
-                            &path,
-                            fcntl::OFlag::O_RDWR
-                                | fcntl::OFlag::O_CLOEXEC
-                                | fcntl::OFlag::O_NOCTTY
-                                | fcntl::OFlag::O_NONBLOCK,
-                        ) {
-                            Ok(fd) => SessionFdDrmDevice(fd),
-                            Err(err) => {
-                                warn!(
-                                    logger,
-                                    "Unable to open drm device {:?}, Error: {:?}. Skipping", path, err
-                                );
-                                return None;
-                            }
-                        }
                 },
-                    logger.clone(),
-                ) {
-                    // Call the handler, which might add it to the runloop
-                    Ok(mut device) => {
-                        let devnum = device.device_id();
-                        let fd = device.as_raw_fd();
-                        match handler.device_added(&mut device) {
-                            Some(drm_handler) => match drm_device_bind(&handle, device, drm_handler) {
-                                Ok((event_source, device)) => Some((devnum, (event_source, device))),
-                                Err((err, mut device)) => {
-                                    warn!(logger, "Failed to bind device. Error: {:?}.", err);
-                                    handler.device_removed(&mut device);
-                                    drop(device);
-                                    if let Err(err) = session.close(fd) {
-                                        warn!(
-                                            logger,
-                                            "Failed to close dropped device. Error: {:?}. Ignoring", err
-                                        );
-                                    };
-                                    None
-                                }
-                            },
-                            None => {
-                                drop(device); //drops master
-                                if let Err(err) = session.close(fd) {
-                                    warn!(logger, "Failed to close device. Error: {:?}. Ignoring", err);
-                                }
-                                None
-                            }
-                        }
-                    }
                 Err(err) => {
-                        warn!(
+                    warn!(log, "Unable to get id of {:?}, Error: {:?}. Skipping", path, err);
-                            logger,
-                            "Failed to initialize device {:?}. Error: {:?}. Skipping", path, err
-                        );
                     None
                 }
-                }
+            })
-            }).collect::<HashMap<dev_t, _>>();
+            .collect();
 
-        let mut builder = MonitorBuilder::new(context).chain_err(|| ErrorKind::FailedToInitMonitor)?;
+        let mut builder = MonitorBuilder::new(context)?;
-        builder
+        builder.match_subsystem("drm")?;
-            .match_subsystem("drm")
+        let monitor = builder.listen()?;
-            .chain_err(|| ErrorKind::FailedToInitMonitor)?;
-        let monitor = builder.listen().chain_err(|| ErrorKind::FailedToInitMonitor)?;
 
         Ok(UdevBackend {
-            _handler: ::std::marker::PhantomData,
+            devices,
-            devices: Rc::new(RefCell::new(devices)),
             monitor,
-            session,
             handler,
-            logger,
+            logger: log,
-            handle,
         })
     }
-
-    /// Closes the udev backend and frees all remaining open devices.
-    pub fn close(&mut self) {
-        let mut devices = self.devices.borrow_mut();
-        for (_, (event_source, device)) in devices.drain() {
-            event_source.remove();
-            let mut device = Rc::try_unwrap(device)
-                .unwrap_or_else(|_| unreachable!())
-                .into_inner();
-            self.handler.device_removed(&mut device);
-            let fd = device.as_raw_fd();
-            drop(device);
-            if let Err(err) = self.session.close(fd) {
-                warn!(self.logger, "Failed to close device. Error: {:?}. Ignoring", err);
-            };
-        }
-        info!(self.logger, "All devices closed");
-    }
 }
 
-impl<
+impl<T: UdevHandler + 'static> Drop for UdevBackend<T>
-        H: DrmHandler<SessionFdDrmDevice> + 'static,
-        S: Session + 'static,
-        T: UdevHandler<H> + 'static,
-        Data: 'static,
-    > Drop for UdevBackend<H, S, T, Data>
 {
     fn drop(&mut self) {
-        self.close();
+        for device in &self.devices {
-    }
+            self.handler.device_removed(*device);
-}
-
-/// `SessionObserver` linked to the `UdevBackend` it was created from.
-pub struct UdevBackendObserver {
-    devices: Weak<
-        RefCell<
-            HashMap<
-                dev_t,
-                (
-                    Source<Generic<EventedRawFd>>,
-                    Rc<RefCell<DrmDevice<SessionFdDrmDevice>>>,
-                ),
-            >,
-        >,
-    >,
-    logger: ::slog::Logger,
-}
-
-impl<
-        H: DrmHandler<SessionFdDrmDevice> + 'static,
-        S: Session + 'static,
-        T: UdevHandler<H> + 'static,
-        Data: 'static,
-    > AsSessionObserver<UdevBackendObserver> for UdevBackend<H, S, T, Data>
-{
-    fn observer(&mut self) -> UdevBackendObserver {
-        UdevBackendObserver {
-            devices: Rc::downgrade(&self.devices),
-            logger: self.logger.clone(),
-        }
-    }
-}
-
-impl SessionObserver for UdevBackendObserver {
-    fn pause(&mut self, devnum: Option<(u32, u32)>) {
-        if let Some(devices) = self.devices.upgrade() {
-            for &mut (_, ref device) in devices.borrow_mut().values_mut() {
-                info!(self.logger, "changed successful");
-                device.borrow_mut().observer().pause(devnum);
-            }
-        }
-    }
-
-    fn activate(&mut self, devnum: Option<(u32, u32, Option<RawFd>)>) {
-        if let Some(devices) = self.devices.upgrade() {
-            for &mut (_, ref device) in devices.borrow_mut().values_mut() {
-                info!(self.logger, "changed successful");
-                device.borrow_mut().observer().activate(devnum);
-            }
         }
     }
 }
@@ -269,122 +102,39 @@ impl SessionObserver for UdevBackendObserver {
 ///
 /// Allows the backend to receive kernel events and thus to drive the `UdevHandler`.
 /// No runtime functionality can be provided without using this function.
-pub fn udev_backend_bind<H, S, T, Data>(
+pub fn udev_backend_bind<T: UdevHandler + 'static, Data: 'static>(
-    mut udev: UdevBackend<H, S, T, Data>,
+    handle: &LoopHandle<Data>,
-) -> ::std::result::Result<Source<Generic<EventedRawFd>>, IoError>
+    udev: UdevBackend<T>,
-where
+) -> Result<Source<Generic<EventedFd<UdevBackend<T>>>>, InsertError<Generic<EventedFd<UdevBackend<T>>>>>
-    H: DrmHandler<SessionFdDrmDevice> + 'static,
-    T: UdevHandler<H> + 'static,
-    S: Session + 'static,
 {
-    let fd = udev.monitor.as_raw_fd();
+    let mut source = Generic::from_fd_source(udev);
-    let handle = udev.handle.clone();
-    let mut source = Generic::from_raw_fd(fd);
     source.set_interest(Ready::readable());
-    handle
+
-        .insert_source(source, move |_, _| {
+    handle.insert_source(source, |evt, _| {
-            udev.process_events();
+        evt.source.borrow_mut().0.process_events();
-        }).map_err(Into::into)
+    })
 }
 
-impl<H, S, T, Data> UdevBackend<H, S, T, Data>
+impl<T: UdevHandler + 'static> UdevBackend<T> {
-where
-    H: DrmHandler<SessionFdDrmDevice> + 'static,
-    T: UdevHandler<H> + 'static,
-    S: Session + 'static,
-    Data: 'static,
-{
     fn process_events(&mut self) {
-        let events = self.monitor.clone().collect::<Vec<Event>>();
+        let monitor = self.monitor.clone();
-        for event in events {
+        for event in monitor {
             match event.event_type() {
                 // New device
                 EventType::Add => {
                     info!(self.logger, "Device Added");
                     if let (Some(path), Some(devnum)) = (event.devnode(), event.devnum()) {
-                        let mut device = {
+                        if self.devices.insert(devnum) {
-                            match DrmDevice::new(
+                            self.handler.device_added(devnum, path.to_path_buf());
-                                {
-                                    let logger = self.logger.clone();
-                                    match self.session.open(
-                                        path,
-                                        fcntl::OFlag::O_RDWR
-                                            | fcntl::OFlag::O_CLOEXEC
-                                            | fcntl::OFlag::O_NOCTTY
-                                            | fcntl::OFlag::O_NONBLOCK,
-                                    ) {
-                                        Ok(fd) => SessionFdDrmDevice(fd),
-                                        Err(err) => {
-                                            warn!(
-                                                logger,
-                                                "Unable to open drm device {:?}, Error: {:?}. Skipping",
-                                                path,
-                                                err
-                                            );
-                                            continue;
                         }
                     }
-                                },
-                                self.logger.clone(),
-                            ) {
-                                Ok(dev) => dev,
-                                Err(err) => {
-                                    warn!(
-                                        self.logger,
-                                        "Failed to initialize device {:?}. Error: {}. Skipping", path, err
-                                    );
-                                    continue;
-                                }
-                            }
-                        };
-                        let fd = device.as_raw_fd();
-                        match self.handler.device_added(&mut device) {
-                            Some(drm_handler) => match drm_device_bind(&self.handle, device, drm_handler) {
-                                Ok(fd_event_source) => {
-                                    self.devices.borrow_mut().insert(devnum, fd_event_source);
-                                }
-                                Err((err, mut device)) => {
-                                    warn!(self.logger, "Failed to bind device. Error: {:?}.", err);
-                                    self.handler.device_removed(&mut device);
-                                    drop(device);
-                                    if let Err(err) = self.session.close(fd) {
-                                        warn!(
-                                            self.logger,
-                                            "Failed to close dropped device. Error: {:?}. Ignoring", err
-                                        );
-                                    };
-                                }
-                            },
-                            None => {
-                                self.handler.device_removed(&mut device);
-                                drop(device);
-                                if let Err(err) = self.session.close(fd) {
-                                    warn!(self.logger, "Failed to close unused device. Error: {:?}", err);
-                                }
-                            }
-                        };
-                    }
                 }
                 // Device removed
                 EventType::Remove => {
                     info!(self.logger, "Device Remove");
                     if let Some(devnum) = event.devnum() {
-                        if let Some((fd_event_source, device)) = self.devices.borrow_mut().remove(&devnum) {
+                        if self.devices.remove(&devnum) {
-                            fd_event_source.remove();
+                            self.handler.device_removed(devnum);
-                            let mut device = Rc::try_unwrap(device)
-                                .unwrap_or_else(|_| unreachable!())
-                                .into_inner();
-                            self.handler.device_removed(&mut device);
-                            let fd = device.as_raw_fd();
-                            drop(device);
-                            if let Err(err) = self.session.close(fd) {
-                                warn!(
-                                    self.logger,
-                                    "Failed to close device {:?}. Error: {:?}. Ignoring",
-                                    event.sysname(),
-                                    err
-                                );
-                            };
                         }
                     }
                 }
@@ -393,9 +143,8 @@ where
                     info!(self.logger, "Device Changed");
                     if let Some(devnum) = event.devnum() {
                         info!(self.logger, "Devnum: {:b}", devnum);
-                        if let Some(&(_, ref device)) = self.devices.borrow_mut().get(&devnum) {
+                        if self.devices.contains(&devnum) {
-                            let handler = &mut self.handler;
+                            self.handler.device_changed(devnum);
-                            handler.device_changed(&mut device.borrow_mut());
                         } else {
                             info!(self.logger, "changed, but device not tracked by backend");
                         };
@@ -409,36 +158,21 @@ where
     }
 }
 
-/// Handler for the `UdevBackend`, allows to open, close and update DRM devices as they change during runtime.
+/// Handler for the `UdevBackend`, allows to open, close and update drm devices as they change during runtime.
-pub trait UdevHandler<H: DrmHandler<SessionFdDrmDevice> + 'static> {
+pub trait UdevHandler {
-    /// Called on initialization for every known device and when a new device is detected.
+    /// Called when a new device is detected.
-    ///
+    fn device_added(&mut self, device: dev_t, path: PathBuf);
-    /// Returning a `DrmHandler` will initialize the device, returning `None` will ignore the device.
-    ///
-    /// ## Panics
-    /// Panics if you try to borrow the token of the belonging `UdevBackend` using this `StateProxy`.
-    fn device_added(&mut self, device: &mut DrmDevice<SessionFdDrmDevice>) -> Option<H>;
     /// Called when an open device is changed.
     ///
     /// This usually indicates that some connectors did become available or were unplugged. The handler
     /// should scan again for connected monitors and mode switch accordingly.
-    ///
+    fn device_changed(&mut self, device: dev_t);
-    /// ## Panics
-    /// Panics if you try to borrow the token of the belonging `UdevBackend` using this `StateProxy`.
-    fn device_changed(&mut self, device: &mut DrmDevice<SessionFdDrmDevice>);
     /// Called when a device was removed.
     ///
-    /// The device will not accept any operations anymore and its file descriptor will be closed once
+    /// The corresponding `UdevRawFd` will never return a valid `RawFd` anymore
-    /// this function returns, any open references/tokens to this device need to be released.
+    /// and its file descriptor will be closed once this function returns,
-    ///
+    /// any open references/tokens to this device need to be released.
-    /// ## Panics
+    fn device_removed(&mut self, device: dev_t);
-    /// Panics if you try to borrow the token of the belonging `UdevBackend` using this `StateProxy`.
-    fn device_removed(&mut self, device: &mut DrmDevice<SessionFdDrmDevice>);
-    /// Called when the udev context has encountered and error.
-    ///
-    /// ## Panics
-    /// Panics if you try to borrow the token of the belonging `UdevBackend` using this `StateProxy`.
-    fn error(&mut self, error: IoError);
 }
 
 /// Returns the path of the primary GPU device if any
@@ -489,22 +223,3 @@ pub fn all_gpus<S: AsRef<str>>(context: &Context, seat: S) -> UdevResult<Vec<Pat
         }).flat_map(|device| device.devnode().map(PathBuf::from))
         .collect())
 }
-
-error_chain! {
-    errors {
-        #[doc = "Failed to scan for devices"]
-        FailedToScan {
-            description("Failed to scan for devices"),
-        }
-
-        #[doc = "Failed to initialize udev monitor"]
-        FailedToInitMonitor {
-            description("Failed to initialize udev monitor"),
-        }
-
-        #[doc = "Failed to identify devices"]
-        FailedToIdentifyDevices {
-            description("Failed to identify devices"),
-        }
-    }
-}