From 4d25153397de707efafa285264874f3739d08ee1 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Tue, 19 Jan 2021 00:36:00 +0000 Subject: [PATCH] Use sh to handle SIGUSR1 and replace fork+exec with spawn --- src/xwayland/launch_helper.rs | 225 ---------------------------------- src/xwayland/mod.rs | 2 - src/xwayland/xserver.rs | 128 ++++++++++--------- 3 files changed, 68 insertions(+), 287 deletions(-) delete mode 100644 src/xwayland/launch_helper.rs diff --git a/src/xwayland/launch_helper.rs b/src/xwayland/launch_helper.rs deleted file mode 100644 index 6e44364..0000000 --- a/src/xwayland/launch_helper.rs +++ /dev/null @@ -1,225 +0,0 @@ -use std::{ - io::{Error as IOError, Read, Result as IOResult, Write}, - os::unix::{ - io::{AsRawFd, FromRawFd, RawFd}, - net::UnixStream, - }, -}; - -use nix::{ - errno::Errno, - libc::exit, - sys::{signal, socket, uio}, - unistd::{fork, ForkResult}, - Error as NixError, Result as NixResult, -}; - -use super::xserver::exec_xwayland; - -const MAX_FDS: usize = 10; - -/// A handle to a `fork()`'d child process that is used for starting XWayland -pub struct LaunchHelper(UnixStream); - -impl LaunchHelper { - /// Start a new launch helper process. - /// - /// # Safety - /// - /// This function calls [`nix::unistd::fork`] to create a new process. This function is unsafe - /// because `fork` is unsafe. Most importantly: Calling `fork` in a multi-threaded process has - /// lots of limitations. Thus, you must call this function before any threads are created. - pub unsafe fn fork() -> IOResult { - let (child, me) = UnixStream::pair()?; - match fork().map_err(nix_error_to_io)? { - ForkResult::Child => { - drop(me); - match do_child(child) { - Ok(()) => exit(0), - Err(e) => { - eprintln!("Error in smithay fork child: {:?}", e); - exit(1); - } - } - } - ForkResult::Parent { child: _ } => Ok(Self(me)), - } - } - - /// Get access to an fd that becomes readable when a launch finished. Call - /// `was_launch_succesful()` once this happens. - pub(crate) fn status_fd(&self) -> RawFd { - self.0.as_raw_fd() - } - - /// Check the status of a previous launch. - pub(crate) fn was_launch_succesful(&self) -> IOResult { - // This reads the one byte that is written at the end of do_child() - let mut buffer = [0]; - let len = (&mut &self.0).read(&mut buffer)?; - Ok(len > 0 && buffer[0] != 0) - } - - /// Fork a child and call exec_xwayland() in it. - pub(crate) fn launch( - &self, - display: u32, - wayland_socket: UnixStream, - wm_socket: UnixStream, - listen_sockets: &[UnixStream], - ) -> IOResult<()> { - let buffer = display.to_ne_bytes(); - let mut fds = vec![wayland_socket.as_raw_fd(), wm_socket.as_raw_fd()]; - fds.extend(listen_sockets.iter().map(|s| s.as_raw_fd())); - assert!(fds.len() <= MAX_FDS); - send_with_fds(self.0.as_raw_fd(), &buffer, &fds) - } -} - -fn do_child(mut stream: UnixStream) -> IOResult<()> { - use nix::sys::wait; - - let mut display = [0; 4]; - let mut fds = [0; MAX_FDS]; - - // Block signals. SIGUSR1 being blocked is inherited by Xwayland and makes it signal its parent - let mut set = signal::SigSet::empty(); - set.add(signal::Signal::SIGUSR1); - set.add(signal::Signal::SIGCHLD); - signal::sigprocmask(signal::SigmaskHow::SIG_BLOCK, Some(&set), None).map_err(nix_error_to_io)?; - - loop { - while wait::waitpid(None, Some(wait::WaitPidFlag::WNOHANG)).is_ok() { - // We just want to reap the zombies - } - - // Receive a new command: u32 display number and associated FDs - let (bytes, num_fds) = receive_fds(stream.as_raw_fd(), &mut display, &mut fds)?; - if bytes == 0 { - // End of file => our parent exited => we should do the same - break Ok(()); - } - - assert_eq!(bytes, 4); - let display = u32::from_ne_bytes(display); - - // Wrap the FDs so that they are later closed. - assert!(num_fds >= 2); - let wayland_socket = unsafe { UnixStream::from_raw_fd(fds[0]) }; - let wm_socket = unsafe { UnixStream::from_raw_fd(fds[1]) }; - let listen_sockets = fds[2..num_fds] - .iter() - .map(|fd| unsafe { UnixStream::from_raw_fd(*fd) }) - .collect::>(); - - // Fork Xwayland and report back the result - let success = match fork_xwayland(display, wayland_socket, wm_socket, &listen_sockets) { - Ok(true) => 1, - Ok(false) => { - eprintln!("Xwayland failed to start"); - 0 - } - Err(e) => { - eprintln!("Failed to fork Xwayland: {:?}", e); - 0 - } - }; - stream.write_all(&[success])?; - } -} - -/// fork() a child process and execute XWayland via exec_xwayland() -fn fork_xwayland( - display: u32, - wayland_socket: UnixStream, - wm_socket: UnixStream, - listen_sockets: &[UnixStream], -) -> NixResult { - match unsafe { fork()? } { - ForkResult::Parent { child: _ } => { - // Wait for the child process to exit or send SIGUSR1 - let mut set = signal::SigSet::empty(); - set.add(signal::Signal::SIGUSR1); - set.add(signal::Signal::SIGCHLD); - match set.wait()? { - signal::Signal::SIGUSR1 => Ok(true), - _ => Ok(false), - } - } - ForkResult::Child => { - match exec_xwayland(display, wayland_socket, wm_socket, listen_sockets) { - Ok(x) => match x {}, - Err(e) => { - // Well, what can we do? Our parent will get SIGCHLD when we exit. - eprintln!("exec Xwayland failed: {:?}", e); - unsafe { exit(1) }; - } - } - } - } -} - -/// Wrapper around `sendmsg()` for FD-passing -fn send_with_fds(fd: RawFd, bytes: &[u8], fds: &[RawFd]) -> IOResult<()> { - let iov = [uio::IoVec::from_slice(bytes)]; - loop { - let result = if !fds.is_empty() { - let cmsgs = [socket::ControlMessage::ScmRights(fds)]; - socket::sendmsg(fd.as_raw_fd(), &iov, &cmsgs, socket::MsgFlags::empty(), None) - } else { - socket::sendmsg(fd.as_raw_fd(), &iov, &[], socket::MsgFlags::empty(), None) - }; - match result { - Ok(len) => { - // All data should have been sent. Why would it fail!? - assert_eq!(len, bytes.len()); - return Ok(()); - } - Err(NixError::Sys(Errno::EINTR)) => { - // Try again - } - Err(e) => return Err(nix_error_to_io(e)), - } - } -} - -/// Wrapper around `recvmsg()` for FD-passing -fn receive_fds(fd: RawFd, buffer: &mut [u8], fds: &mut [RawFd]) -> IOResult<(usize, usize)> { - let mut cmsg = cmsg_space!([RawFd; MAX_FDS]); - let iov = [uio::IoVec::from_mut_slice(buffer)]; - - let msg = loop { - match socket::recvmsg( - fd.as_raw_fd(), - &iov[..], - Some(&mut cmsg), - socket::MsgFlags::empty(), - ) { - Ok(msg) => break msg, - Err(NixError::Sys(Errno::EINTR)) => { - // Try again - } - Err(e) => return Err(nix_error_to_io(e)), - } - }; - - let received_fds = msg.cmsgs().flat_map(|cmsg| match cmsg { - socket::ControlMessageOwned::ScmRights(s) => s, - _ => Vec::new(), - }); - let mut fd_count = 0; - for (fd, place) in received_fds.zip(fds.iter_mut()) { - fd_count += 1; - *place = fd; - } - Ok((msg.bytes, fd_count)) -} - -fn nix_error_to_io(err: NixError) -> IOError { - use std::io::ErrorKind; - match err { - NixError::Sys(errno) => errno.into(), - NixError::InvalidPath | NixError::InvalidUtf8 => IOError::new(ErrorKind::InvalidInput, err), - NixError::UnsupportedOperation => IOError::new(ErrorKind::Other, err), - } -} diff --git a/src/xwayland/mod.rs b/src/xwayland/mod.rs index 1fba973..28782c4 100644 --- a/src/xwayland/mod.rs +++ b/src/xwayland/mod.rs @@ -13,9 +13,7 @@ //! //! Smithay does not provide any helper for doing that yet, but it is planned. -mod launch_helper; mod x11_sockets; mod xserver; -pub use self::launch_helper::LaunchHelper; pub use self::xserver::{XWayland, XWindowManager}; diff --git a/src/xwayland/xserver.rs b/src/xwayland/xserver.rs index 4218696..3e0ddd9 100644 --- a/src/xwayland/xserver.rs +++ b/src/xwayland/xserver.rs @@ -33,11 +33,13 @@ use std::{ any::Any, cell::RefCell, env, - ffi::CString, + io::{Error as IOError, Read, Result as IOResult}, os::unix::{ io::{AsRawFd, IntoRawFd, RawFd}, net::UnixStream, + process::CommandExt, }, + process::{ChildStdout, Command, Stdio}, rc::Rc, sync::Arc, }; @@ -47,14 +49,11 @@ use calloop::{ Interest, LoopHandle, Mode, Source, }; -use nix::Result as NixResult; +use nix::Error as NixError; use wayland_server::{Client, Display, Filter}; -use super::{ - x11_sockets::{prepare_x11_sockets, X11Lock}, - LaunchHelper, -}; +use super::x11_sockets::{prepare_x11_sockets, X11Lock}; /// The XWayland handle pub struct XWayland { @@ -87,7 +86,6 @@ impl XWayland { display: Rc>, data: &mut T, logger: L, - helper: LaunchHelper, ) -> Result, ()> where L: Into>, @@ -113,7 +111,6 @@ impl XWayland { }), wayland_display: display, instance: None, - helper, log: log.new(o!("smithay_module" => "XWayland")), })); launch(&inner, data)?; @@ -133,6 +130,7 @@ struct XWaylandInstance { startup_handler: Option>>, wm_fd: Option, started_at: ::std::time::Instant, + child_stdout: Option, } type SourceMaker = dyn FnMut(Rc>>, RawFd) -> Result>, ()>; @@ -144,7 +142,6 @@ struct Inner { wayland_display: Rc>, instance: Option, kill_source: Box>)>, - helper: LaunchHelper, log: ::slog::Logger, } @@ -185,16 +182,15 @@ fn launch( })); // all is ready, we can do the fork dance - match guard.helper.launch(lock.display(), wl_x11, x_wm_x11, &x_fds) { - Ok(()) => {} + let child_stdout = match spawn_xwayland(lock.display(), wl_x11, x_wm_x11, &x_fds) { + Ok(child_stdout) => child_stdout, Err(e) => { - error!(guard.log, "Could not initiate launch of Xwayland"; "err" => format!("{:?}", e)); + error!(guard.log, "XWayland failed to spawn"; "err" => format!("{:?}", e)); return Err(()); } - } + }; - let status_fd = guard.helper.status_fd(); - let startup_handler = (&mut *guard.source_maker)(inner.clone(), status_fd)?; + let startup_handler = (&mut *guard.source_maker)(inner.clone(), child_stdout.as_raw_fd())?; guard.instance = Some(XWaylandInstance { display_lock: lock, @@ -202,6 +198,7 @@ fn launch( startup_handler: Some(startup_handler), wm_fd: Some(x_wm_me), started_at: creation_time, + child_stdout: Some(child_stdout), }); Ok(()) @@ -258,9 +255,13 @@ fn xwayland_ready(inner: &Rc>>) { // instance should never be None at this point let instance = inner.instance.as_mut().unwrap(); let wm = &mut inner.wm; + // neither the child_stdout + let child_stdout = instance.child_stdout.as_mut().unwrap(); - let success = match inner.helper.was_launch_succesful() { - Ok(s) => s, + // This reads the one byte that is written when sh receives SIGUSR1 + let mut buffer = [0]; + let success = match child_stdout.read(&mut buffer) { + Ok(len) => len > 0 && buffer[0] == b'S', Err(e) => { error!(inner.log, "Checking launch status failed"; "err" => format!("{:?}", e)); false @@ -290,72 +291,79 @@ fn xwayland_ready(inner: &Rc>>) { } } -pub(crate) enum Void {} - /// Exec XWayland with given sockets on given display /// /// If this returns, that means that something failed -pub(crate) fn exec_xwayland( +fn spawn_xwayland( display: u32, wayland_socket: UnixStream, wm_socket: UnixStream, listen_sockets: &[UnixStream], -) -> NixResult { - // uset the CLOEXEC flag from the sockets we need to pass - // to xwayland - unset_cloexec(&wayland_socket)?; - unset_cloexec(&wm_socket)?; +) -> IOResult { + let mut command = Command::new("sh"); + + // We use output stream to communicate because FD is easier to handle than exit code. + command.stdout(Stdio::piped()); + + let mut xwayland_args = format!(":{} -rootless -terminate -wm {}", display, wm_socket.as_raw_fd()); for socket in listen_sockets { - unset_cloexec(socket)?; + xwayland_args.push_str(&format!(" -listen {}", socket.as_raw_fd())); } - // prepare the arguments to XWayland - let mut args = vec![ - CString::new("Xwayland").unwrap(), - CString::new(format!(":{}", display)).unwrap(), - CString::new("-rootless").unwrap(), - CString::new("-terminate").unwrap(), - CString::new("-wm").unwrap(), - CString::new(format!("{}", wm_socket.as_raw_fd())).unwrap(), - ]; - for socket in listen_sockets { - args.push(CString::new("-listen").unwrap()); - args.push(CString::new(format!("{}", socket.as_raw_fd())).unwrap()); - } - // setup the environment: clear everything except PATH and XDG_RUNTIME_DIR - for (key, _) in env::vars_os() { + // This command let sh to: + // * Set up signal handler for USR1 + // * Launch Xwayland with USR1 ignored so Xwayland will signal us when it is ready (also redirect + // Xwayland's STDOUT to STDERR so its output, if any, won't distract us) + // * Print "S" and exit if USR1 is received + command.arg("-c").arg(format!( + "trap 'echo S' USR1; (trap '' USR1; exec Xwayland {}) 1>&2 & wait", + xwayland_args + )); + + // Setup the environment: clear everything except PATH and XDG_RUNTIME_DIR + command.env_clear(); + for (key, value) in env::vars_os() { if key.to_str() == Some("PATH") || key.to_str() == Some("XDG_RUNTIME_DIR") { + command.env(key, value); continue; } - env::remove_var(key); } - // the WAYLAND_SOCKET var tells XWayland where to connect as a wayland client - env::set_var("WAYLAND_SOCKET", format!("{}", wayland_socket.as_raw_fd())); + command.env("WAYLAND_SOCKET", format!("{}", wayland_socket.as_raw_fd())); - // ignore SIGUSR1, this will make the XWayland server send us this - // signal when it is ready apparently unsafe { - use nix::sys::signal::*; - sigaction( - Signal::SIGUSR1, - &SigAction::new(SigHandler::SigIgn, SaFlags::empty(), SigSet::empty()), - )?; + let wayland_socket_fd = wayland_socket.as_raw_fd(); + let wm_socket_fd = wm_socket.as_raw_fd(); + let socket_fds: Vec<_> = listen_sockets.iter().map(|socket| socket.as_raw_fd()).collect(); + command.pre_exec(move || { + // unset the CLOEXEC flag from the sockets we need to pass + // to xwayland + unset_cloexec(wayland_socket_fd)?; + unset_cloexec(wm_socket_fd)?; + for &socket in socket_fds.iter() { + unset_cloexec(socket)?; + } + Ok(()) + }); } - // run it - let ret = ::nix::unistd::execvp( - &CString::new("Xwayland").unwrap(), - &args.iter().map(AsRef::as_ref).collect::>(), - )?; - // small dance to actually return Void - match ret {} + let mut child = command.spawn()?; + Ok(child.stdout.take().expect("stdout should be piped")) +} + +fn nix_error_to_io(err: NixError) -> IOError { + use std::io::ErrorKind; + match err { + NixError::Sys(errno) => errno.into(), + NixError::InvalidPath | NixError::InvalidUtf8 => IOError::new(ErrorKind::InvalidInput, err), + NixError::UnsupportedOperation => IOError::new(ErrorKind::Other, err), + } } /// Remove the `O_CLOEXEC` flag from this `Fd` /// /// This means that the `Fd` will *not* be automatically /// closed when we `exec()` into XWayland -fn unset_cloexec(fd: &F) -> NixResult<()> { +fn unset_cloexec(fd: RawFd) -> IOResult<()> { use nix::fcntl::{fcntl, FcntlArg, FdFlag}; - fcntl(fd.as_raw_fd(), FcntlArg::F_SETFD(FdFlag::empty()))?; + fcntl(fd, FcntlArg::F_SETFD(FdFlag::empty())).map_err(nix_error_to_io)?; Ok(()) }