XWayland: add logging

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<X11Lock, ()> {
+    fn grab(display: u32, log: ::slog::Logger) -> Result<X11Lock, ()> {
+        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);
+                    if let Ok(()) = ::std::fs::remove_file(filename) {
-                    return X11Lock::grab(display);
+                        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));
+        if let Err(e) = ::std::fs::remove_file(format!("/tmp/.X11-unix/X{}", self.display)) {
-        let _ = ::std::fs::remove_file(format!("/tmp/.X{}-lock", 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

src/xwayland/xserver.rs

@@ -66,12 +66,22 @@ pub trait XWindowManager {
 
 impl<WM: XWindowManager + 'static> XWayland<WM> {
     /// Start the XWayland server
-    pub fn init(wm: WM, token: LoopToken, display: Rc<RefCell<Display>>) -> Result<XWayland<WM>, ()> {
+    pub fn init<L>(
+        wm: WM,
+        token: LoopToken,
+        display: Rc<RefCell<Display>>,
+        logger: L,
+    ) -> Result<XWayland<WM>, ()>
+    where
+        L: Into<Option<::slog::Logger>>,
+    {
+        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<WM: XWindowManager> {
     token: LoopToken,
     wayland_display: Rc<RefCell<Display>>,
     instance: Option<XWaylandInstance>,
+    log: ::slog::Logger,
 }
 
 // Launch an XWayland server
@@ -110,10 +121,12 @@ fn launch<WM: XWindowManager + 'static>(inner: &Rc<RefCell<Inner<WM>>>) -> 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<WM: XWindowManager + 'static>(inner: &Rc<RefCell<Inner<WM>>>) -> 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<WM: XWindowManager> Inner<WM> {
     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<WM: XWindowManager + 'static>(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<WM: XWindowManager>(inner: &Rc<RefCell<Inner<WM>>>) {
     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<WM: XWindowManager>(inner: &Rc<RefCell<Inner<WM>>>) {
 
     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<WM: XWindowManager>(inner: &Rc<RefCell<Inner<WM>>>) {
 
         // 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