Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use POSIX shared memory in X11 backend #165

Merged
merged 1 commit into from
Oct 18, 2023
Merged

Use POSIX shared memory in X11 backend #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ harness = false

[features]
default = ["kms", "x11", "x11-dlopen", "wayland", "wayland-dlopen"]
kms = ["bytemuck", "drm", "libc", "rustix"]
kms = ["bytemuck", "drm", "rustix"]
wayland = ["wayland-backend", "wayland-client", "memmap2", "rustix", "fastrand"]
wayland-dlopen = ["wayland-sys/dlopen"]
x11 = ["as-raw-xcb-connection", "bytemuck", "libc", "rustix", "tiny-xlib", "x11rb"]
x11 = ["as-raw-xcb-connection", "bytemuck", "fastrand", "rustix", "tiny-xlib", "x11rb"]
x11-dlopen = ["tiny-xlib/dlopen", "x11rb/dl-libxcb"]

[dependencies]
Expand All @@ -32,18 +32,15 @@ raw-window-handle = "0.5.0"
as-raw-xcb-connection = { version = "1.0.0", optional = true }
bytemuck = { version = "1.12.3", optional = true }
drm = { version = "0.10.0", default-features = false, optional = true }
fastrand = { version = "2.0.0", optional = true }
memmap2 = { version = "0.9.0", optional = true }
libc = { version = "0.2.149", optional = true }
rustix = { version = "0.38.19", features = ["fs", "mm", "shm"], optional = true }
rustix = { version = "0.38.19", features = ["fs", "mm", "shm", "std"], default-features = false, optional = true }
tiny-xlib = { version = "0.2.1", optional = true }
wayland-backend = { version = "0.3.0", features = ["client_system"], optional = true }
wayland-client = { version = "0.31.0", optional = true }
wayland-sys = "0.31.0"
x11rb = { version = "0.12.0", features = ["allow-unsafe-code", "shm"], optional = true }

[target.'cfg(all(unix, not(any(target_vendor = "apple", target_os = "android", target_os = "redox", target_os = "linux", target_os = "freebsd"))))'.dependencies]
fastrand = { version = "2.0.0", optional = true }

[target.'cfg(target_os = "windows")'.dependencies.windows-sys]
version = "0.48.0"
features = ["Win32_Graphics_Gdi", "Win32_UI_WindowsAndMessaging", "Win32_Foundation"]
Expand Down
3 changes: 2 additions & 1 deletion src/kms.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ impl BufferImpl<'_> {
// returns `ENOSYS` and check that before allocating the above and running this.
match self.display.dirty_framebuffer(self.front_fb, &rectangles) {
Ok(()) => {}
Err(drm::SystemError::Unknown { errno }) if errno as i32 == libc::ENOSYS => {}
Err(drm::SystemError::Unknown { errno })
if errno as i32 == rustix::io::Errno::NOSYS.raw_os_error() => {}
Err(e) => {
return Err(SoftBufferError::PlatformError(
Some("failed to dirty framebuffer".into()),
Expand Down
122 changes: 85 additions & 37 deletions src/x11.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@

use crate::error::SwResultExt;
use crate::{Rect, SoftBufferError};
use libc::{shmat, shmctl, shmdt, shmget, IPC_PRIVATE, IPC_RMID};
use raw_window_handle::{XcbDisplayHandle, XcbWindowHandle, XlibDisplayHandle, XlibWindowHandle};
use std::ptr::{null_mut, NonNull};
use rustix::{fd, mm, shm as posix_shm};
use std::{
fmt, io, mem,
fmt,
fs::File,
io, mem,
num::{NonZeroU16, NonZeroU32},
ptr::{null_mut, NonNull},
rc::Rc,
slice,
};
Expand Down Expand Up @@ -606,7 +608,8 @@ impl ShmBuffer {
) -> Result<(), PushBufferError> {
// Register the guard.
let new_id = conn.generate_id()?;
conn.shm_attach(new_id, seg.id(), true)?.ignore_error();
conn.shm_attach_fd(new_id, fd::AsRawFd::as_raw_fd(&seg), true)?
.ignore_error();

// Take out the old one and detach it.
if let Some((old_seg, old_id)) = self.seg.replace((seg, new_id)) {
Expand Down Expand Up @@ -643,7 +646,7 @@ impl ShmBuffer {
}

struct ShmSegment {
id: i32,
id: File,
ptr: NonNull<i8>,
size: usize,
buffer_size: usize,
Expand All @@ -654,32 +657,40 @@ impl ShmSegment {
fn new(size: usize, buffer_size: usize) -> io::Result<Self> {
assert!(size >= buffer_size);

unsafe {
// Create the shared memory segment.
let id = shmget(IPC_PRIVATE, size, 0o600);
if id == -1 {
return Err(io::Error::last_os_error());
}
// Create a shared memory segment.
let id = File::from(create_shm_id()?);

// Map the SHM to our memory space.
let ptr = {
let ptr = shmat(id, null_mut(), 0);
match NonNull::new(ptr as *mut i8) {
Some(ptr) => ptr,
None => {
shmctl(id, IPC_RMID, null_mut());
return Err(io::Error::last_os_error());
}
}
};
// Set its length.
id.set_len(size as u64)?;

Ok(Self {
id,
ptr,
// Map the shared memory to our file descriptor space.
let ptr = unsafe {
let ptr = mm::mmap(
null_mut(),
size,
buffer_size,
})
}
mm::ProtFlags::READ | mm::ProtFlags::WRITE,
mm::MapFlags::SHARED,
&id,
0,
)?;

match NonNull::new(ptr.cast()) {
Some(ptr) => ptr,
None => {
return Err(io::Error::new(
io::ErrorKind::Other,
"unexpected null when mapping SHM segment",
));
}
}
};

Ok(Self {
id,
ptr,
size,
buffer_size,
})
}

/// Get this shared memory segment as a reference.
Expand Down Expand Up @@ -715,21 +726,19 @@ impl ShmSegment {
fn size(&self) -> usize {
self.size
}
}

/// Get the shared memory ID.
fn id(&self) -> u32 {
self.id as _
impl fd::AsRawFd for ShmSegment {
fn as_raw_fd(&self) -> fd::RawFd {
self.id.as_raw_fd()
}
}

impl Drop for ShmSegment {
fn drop(&mut self) {
unsafe {
// Detach the shared memory segment.
shmdt(self.ptr.as_ptr() as _);

// Delete the shared memory segment.
shmctl(self.id, IPC_RMID, null_mut());
// Unmap the shared memory segment.
mm::munmap(self.ptr.as_ptr().cast(), self.size).ok();
}
}
}
Expand Down Expand Up @@ -758,6 +767,45 @@ impl Drop for X11Impl {
}
}

/// Create a shared memory identifier.
fn create_shm_id() -> io::Result<fd::OwnedFd> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should move the create_memfile logic in the wayland backend to somewhere it can be used in both backends, instead of duplicating things.

Using memfd_create is better than shm_open where supported.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if memfiles are supported for X11 MIT-SHM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really find any documentation for ShmAttachFd beyond:

Create a shared memory segment. The file descriptor will be mapped at offset
zero, and the size will be obtained using fstat(). A zero size will result in a
Value error.

The file descriptor the server should mmap().

So any fd it can fstat to get the size and mmap should be fine.

https://lists.freedesktop.org/archives/xorg-devel/2013-October/038490.html

I was hoping for a patch to shmproto.txt, but there isn't one, le sigh.
We should really fix the MIT-SHM docs, Jon's reverse-engineered spec pdf
is full of enough handwave to be embarassing for us.

Lol. I guess the fact https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/blob/master/src/shm.xml exists means the blocker there is addressed, but no one has added documentation for version 1.2 of the protocol yet.

use posix_shm::{Mode, ShmOFlags};

let mut rng = fastrand::Rng::new();
let mut name = String::with_capacity(23);

// Only try four times; the chances of a collision on this space is astronomically low, so if
// we miss four times in a row we're probably under attack.
for i in 0..4 {
name.clear();
name.push_str("softbuffer-x11-");
name.extend(std::iter::repeat_with(|| rng.alphanumeric()).take(7));

// Try to create the shared memory segment.
match posix_shm::shm_open(
&name,
ShmOFlags::RDWR | ShmOFlags::CREATE | ShmOFlags::EXCL,
Mode::RWXU,
) {
Ok(id) => {
posix_shm::shm_unlink(&name).ok();
return Ok(id);
}

Err(rustix::io::Errno::EXIST) => {
log::warn!("x11: SHM ID collision at {} on try number {}", name, i);
}

Err(e) => return Err(e.into()),
};
}

Err(io::Error::new(
io::ErrorKind::Other,
"failed to generate a non-existent SHM name",
))
}

/// Test to see if SHM is available.
fn is_shm_available(c: &impl Connection) -> bool {
// Create a small SHM segment.
Expand All @@ -773,7 +821,7 @@ fn is_shm_available(c: &impl Connection) -> bool {
};

let (attach, detach) = {
let attach = c.shm_attach(seg_id, seg.id(), false);
let attach = c.shm_attach_fd(seg_id, fd::AsRawFd::as_raw_fd(&seg), false);
let detach = c.shm_detach(seg_id);

match (attach, detach) {
Expand Down