From ff63209a17362dee13bdb9de17ee306f53adb94d Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Wed, 2 May 2018 15:54:24 +0200 Subject: [PATCH] XWayland: add logging --- src/xwayland/x11_sockets.rs | 35 ++++++++++++++++++++++------------ src/xwayland/xserver.rs | 38 +++++++++++++++++++++++++++++++------ 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/xwayland/x11_sockets.rs b/src/xwayland/x11_sockets.rs index 1791b68..2fb4486 100644 --- a/src/xwayland/x11_sockets.rs +++ b/src/xwayland/x11_sockets.rs @@ -7,10 +7,10 @@ use nix::errno::Errno; use nix::sys::socket; /// Find a free X11 display slot and setup -pub(crate) fn prepare_x11_sockets() -> Result<(X11Lock, [UnixStream; 2]), ()> { +pub(crate) fn prepare_x11_sockets(log: ::slog::Logger) -> Result<(X11Lock, [UnixStream; 2]), ()> { for d in 0..33 { // if fails, try the next one - if let Ok(lock) = X11Lock::grab(d) { + if let Ok(lock) = X11Lock::grab(d, log.clone()) { // we got a lockfile, try and create the socket if let Ok(sockets) = open_x11_sockets_for_display(d) { return Ok((lock, sockets)); @@ -22,16 +22,15 @@ pub(crate) fn prepare_x11_sockets() -> Result<(X11Lock, [UnixStream; 2]), ()> { return Err(()); } -/// Remove the X11 sockets for a given display number -pub(crate) fn cleanup_x11_sockets(display: u32) {} - pub(crate) struct X11Lock { display: u32, + log: ::slog::Logger, } impl X11Lock { /// Try to grab a lockfile for given X display number - fn grab(display: u32) -> Result { + fn grab(display: u32, log: ::slog::Logger) -> Result { + debug!(log, "Attempting to aquire an X11 display lock"; "D" => display); let filename = format!("/tmp/.X{}-lock", display); let lockfile = ::std::fs::OpenOptions::new() .write(true) @@ -47,11 +46,13 @@ impl X11Lock { let _ = ::std::fs::remove_file(&filename); return Err(()); } else { + debug!(log, "X11 lock aquired"; "D" => display); // we got the lockfile and wrote our pid to it, all is good - return Ok(X11Lock { display }); + return Ok(X11Lock { display, log }); } } Err(_) => { + debug!(log, "Failed to acquire lock"; "D" => display); // 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 @@ -69,8 +70,13 @@ impl X11Lock { 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 let Ok(()) = ::std::fs::remove_file(filename) { + debug!(log, "Lock was blocked by a defunct X11 server, trying again"; "D" => display); + return X11Lock::grab(display, log); + } else { + // we could not remove the lockfile, abort + return Err(()); + } } // if we reach here, this lockfile exists and is probably in use, give up return Err(()); @@ -85,15 +91,20 @@ impl X11Lock { impl Drop for X11Lock { fn drop(&mut self) { + info!(self.log, "Cleaning up X11 lock."); // 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)); + if let Err(e) = ::std::fs::remove_file(format!("/tmp/.X11-unix/X{}", self.display)) { + warn!(self.log, "Failed to remove X11 socket"; "error" => format!("{:?}", e)); + } + if let Err(e) = ::std::fs::remove_file(format!("/tmp/.X{}-lock", self.display)) { + warn!(self.log, "Failed to remove X11 lockfile"; "error" => format!("{:?}", e)); + } } } /// Open the two unix sockets an X server listens on /// -/// SHould only be done after the associated lockfile is aquired! +/// Should only be done after the associated lockfile is aquired! fn open_x11_sockets_for_display(display: u32) -> NixResult<[UnixStream; 2]> { let path = format!("/tmp/.X11-unix/X{}", display); // We know this path is not to long, these unwrap cannot fail diff --git a/src/xwayland/xserver.rs b/src/xwayland/xserver.rs index e8afe5b..3240ff8 100644 --- a/src/xwayland/xserver.rs +++ b/src/xwayland/xserver.rs @@ -66,12 +66,22 @@ pub trait XWindowManager { impl XWayland { /// Start the XWayland server - pub fn init(wm: WM, token: LoopToken, display: Rc>) -> Result, ()> { + pub fn init( + wm: WM, + token: LoopToken, + display: Rc>, + logger: L, + ) -> Result, ()> + where + L: Into>, + { + let log = ::slog_or_stdlog(logger); let inner = Rc::new(RefCell::new(Inner { wm, token, wayland_display: display, instance: None, + log: log.new(o!("smithay_module" => "XWayland")), })); launch(&inner)?; Ok(XWayland { inner }) @@ -99,6 +109,7 @@ struct Inner { token: LoopToken, wayland_display: Rc>, instance: Option, + log: ::slog::Logger, } // Launch an XWayland server @@ -110,10 +121,12 @@ fn launch(inner: &Rc>>) -> Resul return Ok(()); } + info!(guard.log, "Starting XWayland"); + let (x_wm_x11, x_wm_me) = UnixStream::pair().map_err(|_| ())?; let (wl_x11, wl_me) = UnixStream::pair().map_err(|_| ())?; - let (lock, x_fds) = prepare_x11_sockets()?; + let (lock, x_fds) = prepare_x11_sockets(guard.log.clone())?; // we have now created all the required sockets @@ -174,20 +187,20 @@ fn launch(inner: &Rc>>) -> Resul Ok(x) => match x {}, Err(e) => { // well, what can we do ? - eprintln!("[smithay] exec XWayland failed: {:?}", e); + error!(guard.log, "exec XWayland failed"; "err" => format!("{:?}", e)); unsafe { ::nix::libc::exit(1) }; } } } Err(e) => { // well, what can we do ? - eprintln!("[smithay] XWayland second fork failed: {:?}", e); + error!(guard.log, "XWayland second fork failed"; "err" => format!("{:?}", e)); unsafe { ::nix::libc::exit(1) }; } } } Err(e) => { - eprintln!("[smithay] XWayland first fork failed: {:?}", e); + error!(guard.log, "XWayland first fork failed"; "err" => format!("{:?}", e)); return Err(()); } }; @@ -209,6 +222,7 @@ impl Inner { fn shutdown(&mut self) { // don't do anything if not running if let Some(mut instance) = self.instance.take() { + info!(self.log, "Shutting down XWayland."); self.wm.xwayland_exited(); // kill the client instance.wayland_client.kill(); @@ -238,7 +252,13 @@ fn client_destroy(data: *mut ()) { // restart it, unless we really just started it, if it crashes right // at startup there is no point if started_at.map(|t| t.elapsed().as_secs()).unwrap_or(10) > 5 { + warn!(inner.borrow().log, "XWayland crashed, restarting."); let _ = launch(&inner); + } else { + warn!( + inner.borrow().log, + "XWayland crashed less than 5 seconds after its startup, not restarting." + ); } } @@ -253,7 +273,7 @@ fn xwayland_ready(inner: &Rc>>) { let pid = instance.child_pid.unwrap(); // find out if the launch was a success by waiting on the intermediate child - let mut success: bool; + let success: bool; loop { match wait::waitpid(pid, None) { Ok(wait::WaitStatus::Exited(_, 0)) => { @@ -275,6 +295,7 @@ fn xwayland_ready(inner: &Rc>>) { if success { // signal the WM + info!(inner.log, "XWayland is ready, signaling the WM."); wm.xwayland_ready( instance.wm_fd.take().unwrap(), // This is a bug if None instance.wayland_client.clone(), @@ -282,6 +303,11 @@ fn xwayland_ready(inner: &Rc>>) { // setup the environemnt ::std::env::set_var("DISPLAY", format!(":{}", instance.display_lock.display())); + } else { + error!( + inner.log, + "XWayland crashed at startup, will not try to restart it." + ); } // in all cases, cleanup