From 2d8653d9d75ecd014623da2ac92612064f1a0c1a Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Tue, 1 May 2018 12:08:01 +0200 Subject: [PATCH] xwayland: take advantage of RAII for X11 lockfile --- src/xwayland/x11_sockets.rs | 121 +++++++++++++++++++----------------- src/xwayland/xserver.rs | 47 +++++++------- 2 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/xwayland/x11_sockets.rs b/src/xwayland/x11_sockets.rs index 491b365..1791b68 100644 --- a/src/xwayland/x11_sockets.rs +++ b/src/xwayland/x11_sockets.rs @@ -7,19 +7,15 @@ use nix::errno::Errno; use nix::sys::socket; /// Find a free X11 display slot and setup -pub(crate) fn prepare_x11_sockets() -> Result<(u32, [UnixStream; 2]), ()> { +pub(crate) fn prepare_x11_sockets() -> Result<(X11Lock, [UnixStream; 2]), ()> { for d in 0..33 { // if fails, try the next one - if let Err(()) = grab_lockfile(d) { - continue; + if let Ok(lock) = X11Lock::grab(d) { + // we got a lockfile, try and create the socket + if let Ok(sockets) = open_x11_sockets_for_display(d) { + return Ok((lock, sockets)); + } } - // we got a lockfile, try and create the socket - if let Ok(fds) = open_x11_sockets_for_display(d) { - return Ok((d, fds)); - } - // creating the sockets failed, for some readon ? - // release the lockfile and try with the next - release_lockfile(d); } // If we reach here, all values from 0 to 32 failed // we need to stop trying at some point @@ -27,63 +23,72 @@ pub(crate) fn prepare_x11_sockets() -> Result<(u32, [UnixStream; 2]), ()> { } /// Remove the X11 sockets for a given display number -pub(crate) fn cleanup_x11_sockets(display: u32) { - let _ = ::std::fs::remove_file(format!("/tmp/.X11-unix/X{}", display)); - let _ = ::std::fs::remove_file(format!("/tmp/.X{}-lock", display)); +pub(crate) fn cleanup_x11_sockets(display: u32) {} + +pub(crate) struct X11Lock { + display: u32, } -/// Try to grab a lockfile for given X display number -fn grab_lockfile(display: u32) -> Result<(), ()> { - let filename = format!("/tmp/.X{}-lock", display); - let lockfile = ::std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&filename); - match lockfile { - Ok(mut file) => { - // we got it, write our PID in it and we're good - let ret = file.write_fmt(format_args!("{:>10}", ::nix::unistd::Pid::this())); - if let Err(_) = ret { - // write to the file failed ? we abandon +impl X11Lock { + /// Try to grab a lockfile for given X display number + fn grab(display: u32) -> Result { + let filename = format!("/tmp/.X{}-lock", display); + let lockfile = ::std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&filename); + match lockfile { + Ok(mut file) => { + // we got it, write our PID in it and we're good + let ret = file.write_fmt(format_args!("{:>10}", ::nix::unistd::Pid::this())); + if let Err(_) = ret { + // write to the file failed ? we abandon + ::std::mem::drop(file); + let _ = ::std::fs::remove_file(&filename); + return Err(()); + } else { + // we got the lockfile and wrote our pid to it, all is good + return Ok(X11Lock { display }); + } + } + Err(_) => { + // we could not open the file, now we try to read it + // and if it contains the pid of a process that no longer + // exist (so if a previous x server claimed it and did not + // exit gracefully and remove it), we claim it + // if we can't open it, give up + let mut file = ::std::fs::File::open(&filename).map_err(|_| ())?; + let mut spid = [0u8; 11]; + file.read_exact(&mut spid).map_err(|_| ())?; ::std::mem::drop(file); - let _ = ::std::fs::remove_file(&filename); + let pid = ::nix::unistd::Pid::from_raw(::std::str::from_utf8(&spid) + .map_err(|_| ())? + .trim() + .parse::() + .map_err(|_| ())?); + if let Err(NixError::Sys(Errno::ESRCH)) = ::nix::sys::signal::kill(pid, None) { + // no process whose pid equals the contents of the lockfile exists + // remove the lockfile and try grabbing it again + let _ = ::std::fs::remove_file(filename); + return X11Lock::grab(display); + } + // if we reach here, this lockfile exists and is probably in use, give up return Err(()); - } else { - // we got the lockfile and wrote our pid to it, all is good - return Ok(()); } } - Err(_) => { - // we could not open the file, now we try to read it - // and if it contains the pid of a process that no longer - // exist (so if a previous x server claimed it and did not - // exit gracefully and remove it), we claim it - // if we can't open it, give up - let mut file = ::std::fs::File::open(&filename).map_err(|_| ())?; - let mut spid = [0u8; 11]; - file.read_exact(&mut spid).map_err(|_| ())?; - ::std::mem::drop(file); - let pid = ::nix::unistd::Pid::from_raw(::std::str::from_utf8(&spid) - .map_err(|_| ())? - .trim() - .parse::() - .map_err(|_| ())?); - if let Err(NixError::Sys(Errno::ESRCH)) = ::nix::sys::signal::kill(pid, None) { - // no process whose pid equals the contents of the lockfile exists - // remove the lockfile and try grabbing it again - let _ = ::std::fs::remove_file(filename); - return grab_lockfile(display); - } - // if we reach here, this lockfile exists and is probably in use, give up - return Err(()); - } + } + + pub(crate) fn display(&self) -> u32 { + self.display } } -/// Release an X11 lockfile -fn release_lockfile(display: u32) { - let filename = format!("/tmp/.X{}-lock", display); - let _ = ::std::fs::remove_file(filename); +impl Drop for X11Lock { + fn drop(&mut self) { + // Cleanup all the X11 files + let _ = ::std::fs::remove_file(format!("/tmp/.X11-unix/X{}", self.display)); + let _ = ::std::fs::remove_file(format!("/tmp/.X{}-lock", self.display)); + } } /// Open the two unix sockets an X server listens on diff --git a/src/xwayland/xserver.rs b/src/xwayland/xserver.rs index a57a92b..e8afe5b 100644 --- a/src/xwayland/xserver.rs +++ b/src/xwayland/xserver.rs @@ -28,18 +28,18 @@ use std::cell::RefCell; use std::rc::Rc; use std::env; use std::ffi::CString; -use std::os::unix::io::{RawFd, AsRawFd, IntoRawFd}; +use std::os::unix::io::{AsRawFd, IntoRawFd}; use std::os::unix::net::UnixStream; use nix::{Error as NixError, Result as NixResult}; use nix::errno::Errno; -use nix::unistd::{close, fork, ForkResult, Pid}; +use nix::unistd::{fork, ForkResult, Pid}; use nix::sys::signal; use wayland_server::{Client, Display, LoopToken}; use wayland_server::sources::{SignalEvent, Source}; -use super::x11_sockets::{cleanup_x11_sockets, prepare_x11_sockets}; +use super::x11_sockets::{X11Lock, prepare_x11_sockets}; /// The XWayland handle pub struct XWayland { @@ -85,7 +85,7 @@ impl Drop for XWayland { } struct XWaylandInstance { - display: u32, + display_lock: X11Lock, wayland_client: Client, sigusr1_handler: Option>, wm_fd: Option, @@ -113,7 +113,7 @@ fn launch(inner: &Rc>>) -> Resul let (x_wm_x11, x_wm_me) = UnixStream::pair().map_err(|_| ())?; let (wl_x11, wl_me) = UnixStream::pair().map_err(|_| ())?; - let (display, x_fds) = prepare_x11_sockets()?; + let (lock, x_fds) = prepare_x11_sockets()?; // we have now created all the required sockets @@ -121,23 +121,23 @@ fn launch(inner: &Rc>>) -> Resul let creation_time = ::std::time::Instant::now(); // create the wayland client for XWayland - let client = unsafe { guard.wayland_display.borrow_mut().create_client(wl_me.into_raw_fd()) }; + let client = unsafe { + guard + .wayland_display + .borrow_mut() + .create_client(wl_me.into_raw_fd()) + }; client.set_user_data(Rc::into_raw(inner.clone()) as *const () as *mut ()); client.set_destructor(client_destroy::); // setup the SIGUSR1 handler let my_inner = inner.clone(); - let sigusr1_handler = match guard + let sigusr1_handler = guard .token .add_signal_event_source(signal::Signal::SIGUSR1, move |_, ()| { xwayland_ready(&my_inner) - }) { - Ok(v) => v, - Err(_) => { - cleanup_x11_sockets(display); - return Err(()); - } - }; + }) + .map_err(|_| ())?; // all is ready, we can do the fork dance let child_pid = match fork() { @@ -170,7 +170,7 @@ fn launch(inner: &Rc>>) -> Resul } Ok(ForkResult::Child) => { // we are the second child, we exec xwayland - match exec_xwayland(display, wl_x11, x_wm_x11, &x_fds) { + match exec_xwayland(lock.display(), wl_x11, x_wm_x11, &x_fds) { Ok(x) => match x {}, Err(e) => { // well, what can we do ? @@ -188,14 +188,12 @@ fn launch(inner: &Rc>>) -> Resul } Err(e) => { eprintln!("[smithay] XWayland first fork failed: {:?}", e); - // fork failed ? cleanup - cleanup_x11_sockets(display); return Err(()); } }; guard.instance = Some(XWaylandInstance { - display, + display_lock: lock, wayland_client: client, sigusr1_handler: Some(sigusr1_handler), wm_fd: Some(x_wm_me), @@ -218,12 +216,9 @@ impl Inner { if let Some(s) = instance.sigusr1_handler.take() { s.remove(); } - // close the WM fd if it is still there - if let Some(s) = instance.wm_fd.take() { - let _ = s.shutdown(::std::net::Shutdown::Both); - } - // cleanup the X11 sockets - cleanup_x11_sockets(instance.display); + // All connexions and lockfiles are cleaned by their destructors + + // Remove DISPLAY from the env ::std::env::remove_var("DISPLAY"); // We do like wlroots: // > We do not kill the Xwayland process, it dies to broken pipe @@ -282,11 +277,11 @@ fn xwayland_ready(inner: &Rc>>) { // signal the WM wm.xwayland_ready( instance.wm_fd.take().unwrap(), // This is a bug if None - instance.wayland_client.clone() + instance.wayland_client.clone(), ); // setup the environemnt - ::std::env::set_var("DISPLAY", format!(":{}", instance.display)); + ::std::env::set_var("DISPLAY", format!(":{}", instance.display_lock.display())); } // in all cases, cleanup