swapchain: Use `UserDataMap` instead of generic parameter

CHANGELOG.md

@@ -25,6 +25,7 @@
 - `render_texture` was removed from `Frame`, use `render_texture_at` or `render_texture_from_to` instead or use `Gles2Renderer::render_texture` as a direct replacement.
 - Remove `InputBackend::dispatch_new_events`, turning `InputBackend` into a definition of backend event types. Future input backends should be a `calloop::EventSource`.
 - Remove `InputBackend::EventError` associated type as it is unneeded since `dispatch_new_events` was removed.
+- `Swapchain` does not have a generic Userdata-parameter anymore, but utilizes `UserDataMap` instead
 
 ### Additions
 

Cargo.toml

@@ -34,6 +34,7 @@ libc = "0.2.103"
 libseat= { version = "0.1.1", optional = true }
 libloading = { version="0.7.0", optional = true } 
 nix = "0.22"
+once_cell = "1.8.0"
 rand = "0.8.4"
 slog = "2"
 slog-stdlog = { version = "4", optional = true }

examples/raw_drm.rs

@@ -105,9 +105,9 @@ fn main() {
         })
         .collect::<Vec<_>>();
     let mut swapchain = Swapchain::new(allocator, w.into(), h.into(), Fourcc::Argb8888, mods);
-    let first_buffer: Slot<DumbBuffer<FdWrapper>, _> = swapchain.acquire().unwrap().unwrap();
+    let first_buffer: Slot<DumbBuffer<FdWrapper>> = swapchain.acquire().unwrap().unwrap();
     let framebuffer = surface.add_framebuffer(first_buffer.handle(), 32, 32).unwrap();
-    *first_buffer.userdata() = Some(framebuffer);
+    first_buffer.userdata().insert_if_missing(|| framebuffer);
 
     // Get the device as an allocator into the
     let mut vblank_handler = VBlankHandler {
@@ -137,8 +137,8 @@ fn main() {
 }
 
 pub struct VBlankHandler {
-    swapchain: Swapchain<DrmDevice<FdWrapper>, DumbBuffer<FdWrapper>, framebuffer::Handle>,
+    swapchain: Swapchain<DrmDevice<FdWrapper>, DumbBuffer<FdWrapper>>,
-    current: Slot<DumbBuffer<FdWrapper>, framebuffer::Handle>,
+    current: Slot<DumbBuffer<FdWrapper>>,
     surface: Rc<DrmSurface<FdWrapper>>,
 }
 
@@ -147,9 +147,9 @@ impl VBlankHandler {
         {
             // Next buffer
             let next = self.swapchain.acquire().unwrap().unwrap();
-            if next.userdata().is_none() {
+            if next.userdata().get::<framebuffer::Handle>().is_none() {
                 let fb = self.surface.add_framebuffer(next.handle(), 32, 32).unwrap();
-                *next.userdata() = Some(fb);
+                next.userdata().insert_if_missing(|| fb);
             }
 
             // now we could render to the mapping via software rendering.
@@ -165,7 +165,7 @@ impl VBlankHandler {
             self.current = next;
         }
 
-        let fb = self.current.userdata().unwrap();
+        let fb = *self.current.userdata().get::<framebuffer::Handle>().unwrap();
         self.surface
             .page_flip([(fb, self.surface.plane())].iter(), true)
             .unwrap();

src/backend/allocator/swapchain.rs

@@ -1,10 +1,11 @@
 use std::ops::Deref;
 use std::sync::{
     atomic::{AtomicBool, Ordering},
-    Arc, Mutex, MutexGuard,
+    Arc,
 };
 
 use crate::backend::allocator::{Allocator, Buffer, Fourcc, Modifier};
+use crate::utils::user_data::UserDataMap;
 
 pub const SLOT_CAP: usize = 4;
 
@@ -36,7 +37,7 @@ pub const SLOT_CAP: usize = 4;
 /// you can store then in the `Slot`s userdata field. If a buffer is re-used, its userdata is preserved for the next time
 /// it is returned by `acquire()`.
 #[derive(Debug)]
-pub struct Swapchain<A: Allocator<B>, B: Buffer, U: 'static> {
+pub struct Swapchain<A: Allocator<B>, B: Buffer> {
     /// Allocator used by the swapchain
     pub allocator: A,
 
@@ -45,7 +46,7 @@ pub struct Swapchain<A: Allocator<B>, B: Buffer, U: 'static> {
     fourcc: Fourcc,
     modifiers: Vec<Modifier>,
 
-    slots: [Arc<InternalSlot<B, U>>; SLOT_CAP],
+    slots: [Arc<InternalSlot<B>>; SLOT_CAP],
 }
 
 /// Slot of a swapchain containing an allocated buffer and its userdata.
@@ -53,50 +54,49 @@ pub struct Swapchain<A: Allocator<B>, B: Buffer, U: 'static> {
 /// The buffer is marked for re-use once all copies are dropped.
 /// Holding on to this struct will block the buffer in the swapchain.
 #[derive(Debug)]
-pub struct Slot<B: Buffer, U: 'static>(Arc<InternalSlot<B, U>>);
+pub struct Slot<B: Buffer>(Arc<InternalSlot<B>>);
 
 #[derive(Debug)]
-struct InternalSlot<B: Buffer, U: 'static> {
+struct InternalSlot<B: Buffer> {
     buffer: Option<B>,
     acquired: AtomicBool,
-    userdata: Mutex<Option<U>>,
+    userdata: UserDataMap,
 }
 
-impl<B: Buffer, U: 'static> Slot<B, U> {
+impl<B: Buffer> Slot<B> {
     /// Retrieve userdata for this slot.
-    pub fn userdata(&self) -> MutexGuard<'_, Option<U>> {
+    pub fn userdata(&self) -> &UserDataMap {
-        self.0.userdata.lock().unwrap()
+        &self.0.userdata
     }
 }
 
-impl<B: Buffer, U: 'static> Default for InternalSlot<B, U> {
+impl<B: Buffer> Default for InternalSlot<B> {
     fn default() -> Self {
         InternalSlot {
             buffer: None,
             acquired: AtomicBool::new(false),
-            userdata: Mutex::new(None),
+            userdata: UserDataMap::new(),
         }
     }
 }
 
-impl<B: Buffer, U: 'static> Deref for Slot<B, U> {
+impl<B: Buffer> Deref for Slot<B> {
     type Target = B;
     fn deref(&self) -> &B {
         Option::as_ref(&self.0.buffer).unwrap()
     }
 }
 
-impl<B: Buffer, U: 'static> Drop for Slot<B, U> {
+impl<B: Buffer> Drop for Slot<B> {
     fn drop(&mut self) {
         self.0.acquired.store(false, Ordering::SeqCst);
     }
 }
 
-impl<A, B, U> Swapchain<A, B, U>
+impl<A, B> Swapchain<A, B>
 where
     A: Allocator<B>,
     B: Buffer,
-    U: 'static,
 {
     /// Create a new swapchain with the desired allocator, dimensions and pixel format for the created buffers.
     pub fn new(
@@ -105,7 +105,7 @@ where
         height: u32,
         fourcc: Fourcc,
         modifiers: Vec<Modifier>,
-    ) -> Swapchain<A, B, U> {
+    ) -> Swapchain<A, B> {
         Swapchain {
             allocator,
             width,
@@ -120,7 +120,7 @@ where
     ///
     /// The swapchain has an internal maximum of four re-usable buffers.
     /// This function returns the first free one.
-    pub fn acquire(&mut self) -> Result<Option<Slot<B, U>>, A::Error> {
+    pub fn acquire(&mut self) -> Result<Option<Slot<B>>, A::Error> {
         if let Some(free_slot) = self
             .slots
             .iter_mut()

src/backend/drm/surface/gbm.rs

@@ -19,7 +19,7 @@ use slog::{debug, error, o, trace, warn};
 /// Simplified abstraction of a swapchain for gbm-buffers displayed on a [`DrmSurface`].
 pub struct GbmBufferedSurface<D: AsRawFd + 'static> {
     buffers: Buffers<D>,
-    swapchain: Swapchain<GbmDevice<D>, BufferObject<()>, (Dmabuf, FbHandle<D>)>,
+    swapchain: Swapchain<GbmDevice<D>, BufferObject<()>>,
     drm: Arc<DrmSurface<D>>,
 }
 
@@ -127,7 +127,7 @@ where
 
         let mode = drm.pending_mode();
 
-        let mut swapchain: Swapchain<GbmDevice<D>, BufferObject<()>, (Dmabuf, FbHandle<D>)> = Swapchain::new(
+        let mut swapchain: Swapchain<GbmDevice<D>, BufferObject<()>> = Swapchain::new(
             allocator,
             mode.size().0 as u32,
             mode.size().1 as u32,
@@ -148,7 +148,8 @@ where
         let fb = attach_framebuffer(&drm, &*buffer)?;
         let dmabuf = buffer.export()?;
         let handle = fb.fb;
-        *buffer.userdata() = Some((dmabuf, fb));
+        buffer.userdata().insert_if_missing(|| dmabuf);
+        buffer.userdata().insert_if_missing(|| fb);
 
         match drm.test_buffer(handle, &mode, true) {
             Ok(_) => {
@@ -284,14 +285,12 @@ impl<A: AsRawFd + 'static> Drop for FbHandle<A> {
     }
 }
 
-type DmabufSlot<D> = Slot<BufferObject<()>, (Dmabuf, FbHandle<D>)>;
-
 struct Buffers<D: AsRawFd + 'static> {
     drm: Arc<DrmSurface<D>>,
-    _current_fb: DmabufSlot<D>,
+    _current_fb: Slot<BufferObject<()>>,
-    pending_fb: Option<DmabufSlot<D>>,
+    pending_fb: Option<Slot<BufferObject<()>>>,
-    queued_fb: Option<DmabufSlot<D>>,
+    queued_fb: Option<Slot<BufferObject<()>>>,
-    next_fb: Option<DmabufSlot<D>>,
+    next_fb: Option<Slot<BufferObject<()>>>,
 }
 
 // TODO: Replace with #[derive(Debug)] once gbm::BufferObject implements debug
@@ -307,7 +306,7 @@ impl<D> Buffers<D>
 where
     D: AsRawFd + 'static,
 {
-    pub fn new(drm: Arc<DrmSurface<D>>, slot: DmabufSlot<D>) -> Buffers<D> {
+    pub fn new(drm: Arc<DrmSurface<D>>, slot: Slot<BufferObject<()>>) -> Buffers<D> {
         Buffers {
             drm,
             _current_fb: slot,
@@ -319,28 +318,26 @@ where
 
     pub fn next(
         &mut self,
-        swapchain: &mut Swapchain<GbmDevice<D>, BufferObject<()>, (Dmabuf, FbHandle<D>)>,
+        swapchain: &mut Swapchain<GbmDevice<D>, BufferObject<()>>,
     ) -> Result<Dmabuf, Error> {
-        if let Some(slot) = self.next_fb.as_ref() {
+        if self.next_fb.is_none() {
-            return Ok(slot.userdata().as_ref().unwrap().0.clone());
-        }
-
             let slot = swapchain.acquire()?.ok_or(Error::NoFreeSlotsError)?;
 
-        let maybe_buffer = slot.userdata().as_ref().map(|(buf, _)| buf.clone());
+            let maybe_buffer = slot.userdata().get::<Dmabuf>().clone();
-        let dmabuf = match maybe_buffer {
+            if maybe_buffer.is_none() {
-            Some(buf) => buf,
+                let dmabuf = slot.export().map_err(Error::AsDmabufError)?;
-            None => {
-                let dmabuf = slot.export()?;
                 let fb_handle = attach_framebuffer(&self.drm, &*slot)?;
-                *slot.userdata() = Some((dmabuf.clone(), fb_handle));
+
-                dmabuf
+                let userdata = slot.userdata();
+                userdata.insert_if_missing(|| dmabuf);
+                userdata.insert_if_missing(|| fb_handle);
             }
-        };
 
             self.next_fb = Some(slot);
+        }
 
-        Ok(dmabuf)
+        let slot = self.next_fb.as_ref().unwrap();
+        Ok(slot.userdata().get::<Dmabuf>().unwrap().clone())
     }
 
     pub fn queue(&mut self) -> Result<(), Error> {
@@ -367,7 +364,7 @@ where
     fn submit(&mut self) -> Result<(), Error> {
         // yes it does not look like it, but both of these lines should be safe in all cases.
         let slot = self.queued_fb.take().unwrap();
-        let fb = slot.userdata().as_ref().unwrap().1.fb;
+        let fb = slot.userdata().get::<FbHandle<D>>().unwrap().fb;
 
         let flip = if self.drm.commit_pending() {
             self.drm.commit([(fb, self.drm.plane())].iter(), true)

src/utils/mod.rs

@@ -6,6 +6,8 @@ pub mod signaling;
 #[cfg(feature = "x11rb_event_source")]
 pub mod x11rb;
 
+pub mod user_data;
+
 pub use self::geometry::{Buffer, Logical, Physical, Point, Raw, Rectangle, Size};
 
 /// This resource is not managed by Smithay

src/utils/user_data.rs

@@ -0,0 +1,324 @@
+//! Various utilities used for user data implementations
+
+use once_cell::sync::OnceCell;
+
+use std::any::Any;
+use std::mem::ManuallyDrop;
+use std::thread::{self, ThreadId};
+
+use self::list::AppendList;
+
+/// A wrapper for user data, able to store any type, and correctly
+/// handling access from a wrong thread
+#[derive(Debug)]
+pub struct UserData {
+    inner: OnceCell<UserDataInner>,
+}
+
+#[derive(Debug)]
+enum UserDataInner {
+    ThreadSafe(Box<dyn Any + Send + Sync + 'static>),
+    NonThreadSafe(Box<ManuallyDrop<dyn Any + 'static>>, ThreadId),
+}
+
+// UserData itself is always threadsafe, as it only gives access to its
+// content if it is send+sync or we are on the right thread
+unsafe impl Send for UserData {}
+unsafe impl Sync for UserData {}
+
+impl UserData {
+    /// Create a new UserData instance
+    pub const fn new() -> UserData {
+        UserData {
+            inner: OnceCell::new(),
+        }
+    }
+
+    /// Sets the UserData to a given value
+    ///
+    /// The provided closure is called to init the UserData,
+    /// does nothing is the UserData had already been set.
+    pub fn set<T: Any + 'static, F: FnOnce() -> T>(&self, f: F) {
+        self.inner.get_or_init(|| {
+            UserDataInner::NonThreadSafe(Box::new(ManuallyDrop::new(f())), thread::current().id())
+        });
+    }
+
+    /// Sets the UserData to a given threadsafe value
+    ///
+    /// The provided closure is called to init the UserData,
+    /// does nothing is the UserData had already been set.
+    pub fn set_threadsafe<T: Any + Send + Sync + 'static, F: FnOnce() -> T>(&self, f: F) {
+        self.inner
+            .get_or_init(|| UserDataInner::ThreadSafe(Box::new(f())));
+    }
+
+    /// Attempt to access the wrapped user data
+    ///
+    /// Will return `None` if either:
+    ///
+    /// - The requested type `T` does not match the type used for construction
+    /// - This `UserData` has been created using the non-threadsafe variant and access
+    ///   is attempted from an other thread than the one it was created on
+    pub fn get<T: 'static>(&self) -> Option<&T> {
+        match self.inner.get() {
+            Some(&UserDataInner::ThreadSafe(ref val)) => <dyn Any>::downcast_ref::<T>(&**val),
+            Some(&UserDataInner::NonThreadSafe(ref val, threadid)) => {
+                // only give access if we are on the right thread
+                if threadid == thread::current().id() {
+                    <dyn Any>::downcast_ref::<T>(&***val)
+                } else {
+                    None
+                }
+            }
+            None => None,
+        }
+    }
+}
+
+impl Drop for UserData {
+    fn drop(&mut self) {
+        // only drop non-Send user data if we are on the right thread, leak it otherwise
+        if let Some(&mut UserDataInner::NonThreadSafe(ref mut val, threadid)) = self.inner.get_mut() {
+            if threadid == thread::current().id() {
+                unsafe {
+                    ManuallyDrop::drop(&mut **val);
+                }
+            }
+        }
+    }
+}
+
+/// A storage able to store several values of `UserData`
+/// of different types. It behaves similarly to a `TypeMap`.
+#[derive(Debug)]
+pub struct UserDataMap {
+    list: AppendList<UserData>,
+}
+
+impl UserDataMap {
+    /// Create a new map
+    pub fn new() -> UserDataMap {
+        UserDataMap {
+            list: AppendList::new(),
+        }
+    }
+
+    /// Attempt to access the wrapped user data of a given type
+    ///
+    /// Will return `None` if no value of type `T` is stored in this `UserDataMap`
+    /// and accessible from this thread
+    pub fn get<T: 'static>(&self) -> Option<&T> {
+        for user_data in &self.list {
+            if let Some(val) = user_data.get::<T>() {
+                return Some(val);
+            }
+        }
+        None
+    }
+
+    /// Insert a value in the map if it is not already there
+    ///
+    /// This is the non-threadsafe variant, the type you insert don't have to be
+    /// threadsafe, but they will not be visible from other threads (even if they are
+    /// actually threadsafe).
+    ///
+    /// If the value does not already exists, the closure is called to create it and
+    /// this function returns `true`. If the value already exists, the closure is not
+    /// called, and this function returns `false`.
+    pub fn insert_if_missing<T: 'static, F: FnOnce() -> T>(&self, init: F) -> bool {
+        if self.get::<T>().is_some() {
+            return false;
+        }
+        let data = UserData::new();
+        data.set(init);
+        self.list.append(data);
+        true
+    }
+
+    /// Insert a value in the map if it is not already there
+    ///
+    /// This is the threadsafe variant, the type you insert must be threadsafe and will
+    /// be visible from all threads.
+    ///
+    /// If the value does not already exists, the closure is called to create it and
+    /// this function returns `true`. If the value already exists, the closure is not
+    /// called, and this function returns `false`.
+    pub fn insert_if_missing_threadsafe<T: Send + Sync + 'static, F: FnOnce() -> T>(&self, init: F) -> bool {
+        if self.get::<T>().is_some() {
+            return false;
+        }
+        let data = UserData::new();
+        data.set_threadsafe(init);
+        self.list.append(data);
+        true
+    }
+}
+
+impl Default for UserDataMap {
+    fn default() -> UserDataMap {
+        UserDataMap::new()
+    }
+}
+
+mod list {
+    /*
+     * This is a lock-free append-only list, it is used as an implementation
+     * detail of the UserDataMap.
+     *
+     * It was extracted from https://github.com/Diggsey/lockless under MIT license
+     * Copyright © Diggory Blake <diggsey@googlemail.com>
+     */
+
+    use std::sync::atomic::{AtomicPtr, Ordering};
+    use std::{mem, ptr};
+
+    type NodePtr<T> = Option<Box<Node<T>>>;
+
+    #[derive(Debug)]
+    struct Node<T> {
+        value: T,
+        next: AppendList<T>,
+    }
+
+    #[derive(Debug)]
+    pub struct AppendList<T>(AtomicPtr<Node<T>>);
+
+    impl<T> AppendList<T> {
+        fn node_into_raw(ptr: NodePtr<T>) -> *mut Node<T> {
+            match ptr {
+                Some(b) => Box::into_raw(b),
+                None => ptr::null_mut(),
+            }
+        }
+        unsafe fn node_from_raw(ptr: *mut Node<T>) -> NodePtr<T> {
+            if ptr.is_null() {
+                None
+            } else {
+                Some(Box::from_raw(ptr))
+            }
+        }
+
+        fn new_internal(ptr: NodePtr<T>) -> Self {
+            AppendList(AtomicPtr::new(Self::node_into_raw(ptr)))
+        }
+
+        pub fn new() -> Self {
+            Self::new_internal(None)
+        }
+
+        pub fn append(&self, value: T) {
+            self.append_list(AppendList::new_internal(Some(Box::new(Node {
+                value,
+                next: AppendList::new(),
+            }))));
+        }
+
+        unsafe fn append_ptr(&self, p: *mut Node<T>) {
+            loop {
+                match self
+                    .0
+                    .compare_exchange_weak(ptr::null_mut(), p, Ordering::AcqRel, Ordering::Acquire)
+                {
+                    Ok(_) => return,
+                    Err(head) => {
+                        if !head.is_null() {
+                            return (*head).next.append_ptr(p);
+                        }
+                    }
+                }
+            }
+        }
+
+        pub fn append_list(&self, other: AppendList<T>) {
+            let p = other.0.load(Ordering::Acquire);
+            mem::forget(other);
+            unsafe { self.append_ptr(p) };
+        }
+
+        pub fn iter(&self) -> AppendListIterator<'_, T> {
+            AppendListIterator(&self.0)
+        }
+
+        pub fn iter_mut(&mut self) -> AppendListMutIterator<'_, T> {
+            AppendListMutIterator(&mut self.0)
+        }
+    }
+
+    impl<'a, T> IntoIterator for &'a AppendList<T> {
+        type Item = &'a T;
+        type IntoIter = AppendListIterator<'a, T>;
+
+        fn into_iter(self) -> AppendListIterator<'a, T> {
+            self.iter()
+        }
+    }
+
+    impl<'a, T> IntoIterator for &'a mut AppendList<T> {
+        type Item = &'a mut T;
+        type IntoIter = AppendListMutIterator<'a, T>;
+
+        fn into_iter(self) -> AppendListMutIterator<'a, T> {
+            self.iter_mut()
+        }
+    }
+
+    impl<T> Drop for AppendList<T> {
+        fn drop(&mut self) {
+            unsafe { Self::node_from_raw(mem::replace(self.0.get_mut(), ptr::null_mut())) };
+        }
+    }
+
+    #[derive(Debug)]
+    pub struct AppendListIterator<'a, T>(&'a AtomicPtr<Node<T>>);
+
+    impl<'a, T: 'a> Iterator for AppendListIterator<'a, T> {
+        type Item = &'a T;
+
+        fn next(&mut self) -> Option<&'a T> {
+            let p = self.0.load(Ordering::Acquire);
+            if p.is_null() {
+                None
+            } else {
+                unsafe {
+                    self.0 = &(*p).next.0;
+                    Some(&(*p).value)
+                }
+            }
+        }
+    }
+
+    #[derive(Debug)]
+    pub struct AppendListMutIterator<'a, T>(&'a mut AtomicPtr<Node<T>>);
+
+    impl<'a, T: 'a> Iterator for AppendListMutIterator<'a, T> {
+        type Item = &'a mut T;
+
+        fn next(&mut self) -> Option<&'a mut T> {
+            let p = self.0.load(Ordering::Acquire);
+            if p.is_null() {
+                None
+            } else {
+                unsafe {
+                    self.0 = &mut (*p).next.0;
+                    Some(&mut (*p).value)
+                }
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::UserDataMap;
+
+    #[test]
+    fn insert_twice() {
+        let map = UserDataMap::new();
+
+        assert_eq!(map.get::<usize>(), None);
+        assert!(map.insert_if_missing(|| 42usize));
+        assert!(!map.insert_if_missing(|| 43usize));
+        assert_eq!(map.get::<usize>(), Some(&42));
+    }
+}