search

Smithay / wayland-window

A simple window-decorations library built on top of wayland-client.
MIT License
18 stars 11 forks source link

Use memfd instead of temporary files #14

Open sp3d opened 7 years ago

sp3d commented 7 years ago

Temporary files are essentially a hack. For the purposes of shm, there's no need to have any filesystem entry, and it's possible for the tmp directory to be disk-backed or otherwise persistent. Linux (which is the only platform supporting Wayland properly at present) provides memfd which is generally agreed upon to be the right way to get an fd referencing a memory region. Memfd is also useful for compositors to ensure they can use mapped memory without worrying about the mapping being shrunk out from under them by buggy or hostile applications; see https://lwn.net/Articles/591108/.

Avoiding the tempfile crate will also shrink the transitive dependency graph significantly.

elinorbgr commented 7 years ago

Is there an already existing rust wrapper around memfd, or would that mean handling the syscall manually in wayland-window?

sp3d commented 7 years ago

I don't see anything on crates.io, but there seems to be https://github.com/sdroege/memfd-rs (which looks like it presents a reasonable safe abstraction around memfd in general). Maybe ask the author if there's anything blocking a published release.

elinorbgr commented 7 years ago

Okay, I've been thinking a little about this, and there are some things that concerns me:

sp3d commented 7 years ago

The memfd syscall is Linux-only. Wayland is Linux-first but should fundamentally be implementable on any OS, and I'm happy to see FreeBSD support moving along. From a cursory look, it seems that the FreeBSD equivalent of memfd is the FreeBSD-specific SHM_ANON flag for shm_open (though I'm not sure if there's any analogue for sealing).

For simple usage, it may be easiest to just use nix::sys::memfd::* directly.

Compositors are in the position of needing to support the sloppiest-implemented client users will want to run; most will support tempfile-backed shm as well as a million other features (e.g. X11 compatibility), but the beauty of the Wayland protocol is that it doesn't require all that complexity, and use-case-specific compositors can be much stricter and simpler than the DEs that most users run on their desktops. Clients can be made simpler and more robust by avoiding dependence on the filesystem (even open() with the Linux O_TMPFILE option involves creating an inode in the underlying filesystem which isn't guaranteed to be tmpfs) for their shared-memory mappings.

Of course this isn't a high-priority bug, since user-facing functionality shouldn't change noticeably, but it would be a nice change that reduces the total number of moving parts in the stack as a whole.

valpackett commented 6 years ago

Yep, SHM_ANON is the way to go on FreeBSD, I recently added that constant to rust libc and added its usage to ipc-channel (waiting for a libc release). Maybe we need to extract that create_shmem code into a new crate.

What does sealing mean?

valpackett commented 6 years ago

Published the tiny memfd/SHM_ANON wrapper crate! https://github.com/myfreeweb/shmemfdrs

elinorbgr commented 6 years ago

@myfreeweb that's an interesting crate, thanks for notifying!

Just to make sure is the returned c_int from it a regular file descriptor, that can be given to File::from_raw_fd(..) and used like a regular rust File?

If so, would it make sense to integrate this casting in your API, making it usable with a safe interface?

valpackett commented 6 years ago

It was extracted from Servo's ipc-channel and they use a raw c_int… I didn't want to change their code much.

sp3d commented 6 years ago

@myfreeweb, sealing is well-described at https://dvdhrm.wordpress.com/tag/memfd/. For Wayland IPC the main use-case is SEAL_SHRINK to prevent clients from causing bus errors in the server when operating on mmap'd regions that the client shrinks out from under the server.