Use -displayfd instead of signals for Xwayland readiness

The -displayfd argument is much friendlier to testing and is generally
more stable. As a result, omitting the display argument to the program
will result in Xwayland searching for the next available display. This
also allows integration tests to run simultaneously and when there is
already an X server running on the system.

This commit also changes how the state of the program is communicated
with integration tests (via the RunData trait).

Cargo.lock

@@ -434,25 +434,6 @@ version = "1.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64"
 
-[[package]]
-name = "signal-hook"
-version = "0.3.17"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8621587d4798caf8eb44879d42e56b9a93ea5dcd315a6487c357130095b62801"
-dependencies = [
- "libc",
- "signal-hook-registry",
-]
-
-[[package]]
-name = "signal-hook-registry"
-version = "1.4.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a9e9e0b4211b72e7b8b6e85c807d36c212bdb33ea8587f7569562a84df5465b1"
-dependencies = [
- "libc",
-]
-
 [[package]]
 name = "slotmap"
 version = "1.0.7"
@@ -734,7 +715,6 @@ dependencies = [
  "paste",
  "pretty_env_logger",
  "rustix",
- "signal-hook",
  "slotmap",
  "testwl",
  "wayland-client",

satellite/Cargo.toml

@@ -18,7 +18,6 @@ wayland-scanner = "0.31.1"
 wayland-server = "0.31.1"
 xcb = { version = "1.3.0", features = ["composite"] }
 wl_drm = { path = "../wl_drm" }
-signal-hook = "0.3.17"
 libc = "0.2.153"
 log = "0.4.21"
 env_logger = "0.11.3"

satellite/src/lib.rs

@@ -6,17 +6,11 @@ use crate::server::{PendingSurfaceState, ServerState};
 use crate::xstate::XState;
 use log::{error, info};
 use rustix::event::{poll, PollFd, PollFlags};
-use signal_hook::consts::*;
 use std::io::{BufRead, BufReader, Read, Write};
-use std::mem::MaybeUninit;
 use std::os::fd::{AsFd, AsRawFd, BorrowedFd};
-use std::os::unix::{net::UnixStream, process::CommandExt};
+use std::os::unix::net::UnixStream;
 use std::process::{Command, Stdio};
-use std::sync::{
+use std::sync::Arc;
-    atomic::{AtomicBool, Ordering},
-    mpsc::Sender,
-    Arc,
-};
 use wayland_server::{Display, ListeningSocket};
 use xcb::x;
 
@@ -36,62 +30,46 @@ pub trait FromServerState<C: XConnection> {
 
 type RealServerState = ServerState<Arc<xcb::Connection>>;
 
-#[derive(Debug, PartialEq, Eq)]
+pub trait RunData {
-pub enum StateEvent {
+    fn display(&self) -> Option<&str>;
-    CreatedServer,
+    fn server(&self) -> Option<UnixStream> {
-    ConnectedServer,
+        None
-    XwaylandReady,
+    }
+    fn created_server(&self) {}
+    fn connected_server(&self) {}
+    fn xwayland_ready(&self, _display: String) {}
 }
 
-pub fn main(comp: Option<UnixStream>, state_updater: Option<Sender<StateEvent>>) -> Option<()> {
+pub fn main(data: impl RunData) -> Option<()> {
-    let display_arg = get_display()?;
-
     let socket = ListeningSocket::bind_auto("wayland", 1..=128).unwrap();
     let mut display = Display::<RealServerState>::new().unwrap();
     let dh = display.handle();
+    data.created_server();
 
-    let mut server_state = RealServerState::new(dh, comp);
+    let mut server_state = RealServerState::new(dh, data.server());
-    if let Some(ref s) = state_updater {
-        s.send(StateEvent::CreatedServer).unwrap();
-    }
 
     let (xsock_wl, xsock_xwl) = UnixStream::pair().unwrap();
     // Prevent creation of new Xwayland command from closing fd
     rustix::io::fcntl_setfd(&xsock_xwl, rustix::io::FdFlags::empty()).unwrap();
 
-    // Flag when Xwayland is ready to accept our connection
+    let (ready_tx, ready_rx) = UnixStream::pair().unwrap();
-    let ready = Arc::new(AtomicBool::new(false));
+    rustix::io::fcntl_setfd(&ready_tx, rustix::io::FdFlags::empty()).unwrap();
-    signal_hook::flag::register(SIGUSR1, ready.clone()).unwrap();
-
     let mut xwayland = Command::new("Xwayland");
-    let mut xwayland = unsafe {
+    if let Some(display) = data.display() {
-        xwayland.pre_exec(|| {
+        xwayland.arg(display);
-            // Set SIGUSR1 to SIG_IGN for Xwayland to get SIGUSR1 to our main process,
-            // which signifies that the X server is ready to accept connections
-            let mut sa_mask = MaybeUninit::uninit();
-            libc::sigemptyset(sa_mask.as_mut_ptr());
-            let sa_mask = sa_mask.assume_init();
-            let act = libc::sigaction {
-                sa_sigaction: libc::SIG_IGN,
-                sa_mask,
-                sa_flags: 0,
-                sa_restorer: None,
-            };
-            libc::sigaction(SIGUSR1, &act, std::ptr::null_mut());
-            Ok(())
-        })
     }
-    .env("WAYLAND_DISPLAY", socket.socket_name().unwrap())
+    let mut xwayland = xwayland
-    //.env("WAYLAND_DEBUG", "1")
+        .args([
-    .args([
+            "-rootless",
-        &display_arg,
+            "-wm",
-        "-rootless",
+            &xsock_xwl.as_raw_fd().to_string(),
-        "-wm",
+            "-displayfd",
-        &format!("{}", &xsock_xwl.as_raw_fd()),
+            &ready_tx.as_raw_fd().to_string(),
-    ])
+        ])
-    .stderr(Stdio::piped())
+        .env("WAYLAND_DISPLAY", socket.socket_name().unwrap())
-    .spawn()
+        .stderr(Stdio::piped())
-    .unwrap();
+        .spawn()
+        .unwrap();
 
     let (mut finish_tx, mut finish_rx) = UnixStream::pair().unwrap();
     let stderr = xwayland.stderr.take().unwrap();
@@ -124,9 +102,7 @@ pub fn main(comp: Option<UnixStream>, state_updater: Option<Sender<StateEvent>>)
                 return None;
             }
 
-            if let Some(ref s) = state_updater {
+            data.connected_server();
-                s.send(StateEvent::ConnectedServer).unwrap();
-            }
             socket.accept().unwrap().unwrap()
         }
         Err(e) => {
@@ -149,26 +125,29 @@ pub fn main(comp: Option<UnixStream>, state_updater: Option<Sender<StateEvent>>)
         PollFd::from_borrowed_fd(server_fd, PollFlags::IN),
         PollFd::new(&xsock_wl, PollFlags::IN),
         PollFd::from_borrowed_fd(display_fd, PollFlags::IN),
+        PollFd::new(&ready_rx, PollFlags::IN),
     ];
 
+    let mut ready = false;
     loop {
         match poll(&mut fds, -1) {
-            Ok(_) => {}
+            Ok(_) => {
-            Err(rustix::io::Errno::INTR) => {
+                if !fds[3].revents().is_empty() {
-                // Typically caused by SIGUSR1
+                    ready = true;
-                if !ready.load(Ordering::Relaxed) {
-                    continue;
                 }
             }
             Err(other) => panic!("Poll failed: {other:?}"),
         }
 
-        if xstate.is_none() && ready.load(Ordering::Relaxed) {
+        if xstate.is_none() && ready {
             let xstate = xstate.insert(XState::new(xsock_wl.as_fd()));
-            info!("Connected to Xwayland on {display_arg}");
+            let mut reader = BufReader::new(&ready_rx);
-            if let Some(ref s) = state_updater {
+            let mut display = String::new();
-                s.send(StateEvent::XwaylandReady).unwrap();
+            reader.read_line(&mut display).unwrap();
-            }
+            display.pop();
+            display.insert(0, ':');
+            info!("Connected to Xwayland on {display}");
+            data.xwayland_ready(display);
             server_state.set_x_connection(xstate.connection.clone());
             server_state.atoms = Some(xstate.atoms.clone());
         }
@@ -182,16 +161,3 @@ pub fn main(comp: Option<UnixStream>, state_updater: Option<Sender<StateEvent>>)
         display.flush_clients().unwrap();
     }
 }
-
-fn get_display() -> Option<String> {
-    let mut args: Vec<_> = std::env::args().collect();
-    if args.len() > 2 {
-        error!("Unexpected arguments: {:?}", &args[2..]);
-        return None;
-    }
-    if args.len() == 1 {
-        Some(":0".into())
-    } else {
-        Some(args.swap_remove(1))
-    }
-}

satellite/src/main.rs

@@ -3,5 +3,22 @@ fn main() {
         .filter_level(log::LevelFilter::Info)
         .parse_default_env()
         .init();
-    xwayland_satellite::main(None, None);
+    xwayland_satellite::main(RealData(get_display()));
+}
+
+#[repr(transparent)]
+struct RealData(Option<String>);
+impl xwayland_satellite::RunData for RealData {
+    fn display(&self) -> Option<&str> {
+        self.0.as_deref()
+    }
+}
+
+fn get_display() -> Option<String> {
+    let mut args: Vec<_> = std::env::args().collect();
+    if args.len() > 2 {
+        panic!("Unexpected arguments: {:?}", &args[2..]);
+    }
+
+    (args.len() == 2).then(|| args.swap_remove(1))
 }

satellite/tests/integration.rs

@@ -2,19 +2,73 @@ use rustix::event::{poll, PollFd, PollFlags};
 use std::mem::ManuallyDrop;
 use std::os::fd::{AsRawFd, BorrowedFd};
 use std::os::unix::net::UnixStream;
-use std::sync::mpsc;
+use std::sync::{
-use std::sync::Once;
+    atomic::{AtomicBool, Ordering},
+    Arc, Mutex, Once,
+};
 use std::thread::JoinHandle;
-use std::time::Duration;
+use std::time::{Duration, Instant};
 use wayland_protocols::xdg::shell::server::xdg_toplevel;
 use wayland_server::Resource;
 use xcb::{x, Xid};
 use xwayland_satellite as xwls;
 
+#[derive(Default)]
+struct TestDataInner {
+    server_created: AtomicBool,
+    server_connected: AtomicBool,
+    display: Mutex<Option<String>>,
+    server: Mutex<Option<UnixStream>>,
+}
+
+#[derive(Default, Clone)]
+struct TestData(Arc<TestDataInner>);
+
+impl TestData {
+    fn new(server: UnixStream) -> Self {
+        Self(Arc::new(TestDataInner {
+            server: Mutex::new(server.into()),
+            ..Default::default()
+        }))
+    }
+}
+
+impl std::ops::Deref for TestData {
+    type Target = Arc<TestDataInner>;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl xwls::RunData for TestData {
+    fn created_server(&self) {
+        self.server_created.store(true, Ordering::Relaxed);
+    }
+
+    fn connected_server(&self) {
+        self.server_connected.store(true, Ordering::Relaxed);
+    }
+
+    fn xwayland_ready(&self, display: String) {
+        *self.display.lock().unwrap() = Some(display);
+    }
+
+    fn display(&self) -> Option<&str> {
+        None
+    }
+
+    fn server(&self) -> Option<UnixStream> {
+        let mut server = self.server.lock().unwrap();
+        assert!(server.is_some());
+        server.take()
+    }
+}
+
 struct Fixture {
     testwl: testwl::Server,
     thread: ManuallyDrop<JoinHandle<Option<()>>>,
     pollfd: PollFd<'static>,
+    display: String,
 }
 
 impl Drop for Fixture {
@@ -26,19 +80,9 @@ impl Drop for Fixture {
     }
 }
 
-xcb::atoms_struct! {
-    struct Atoms {
-        wm_protocols => b"WM_PROTOCOLS",
-        wm_delete_window => b"WM_DELETE_WINDOW",
-        wm_class => b"WM_CLASS",
-        wm_name => b"WM_NAME",
-    }
-}
-
-static INIT: Once = Once::new();
 impl Fixture {
-    #[track_caller]
     fn new() -> Self {
+        static INIT: Once = Once::new();
         INIT.call_once(|| {
             env_logger::builder()
                 .is_test(true)
@@ -49,9 +93,9 @@ impl Fixture {
         let (a, b) = UnixStream::pair().unwrap();
         let mut testwl = testwl::Server::new(false);
         testwl.connect(a);
-
+        let our_data = TestData::new(b);
-        let (send, recv) = mpsc::channel();
+        let data = our_data.clone();
-        let thread = std::thread::spawn(move || xwls::main(Some(b), Some(send)));
+        let thread = std::thread::spawn(move || xwls::main(data));
 
         // wait for connection
         let fd = unsafe { BorrowedFd::borrow_raw(testwl.poll_fd().as_raw_fd()) };
@@ -59,38 +103,46 @@ impl Fixture {
         assert!(poll(&mut [pollfd.clone()], 100).unwrap() > 0);
         testwl.dispatch();
 
-        let wait = Duration::from_secs(1);
+        let try_bool_timeout = |b: &AtomicBool| {
-        assert_eq!(
+            let timeout = Duration::from_secs(1);
-            recv.recv_timeout(wait),
+            let mut res = b.load(Ordering::Relaxed);
-            Ok(xwls::StateEvent::CreatedServer),
+            let start = Instant::now();
+            while !res && start.elapsed() < timeout {
+                res = b.load(Ordering::Relaxed);
+            }
+
+            res
+        };
+        assert!(
+            try_bool_timeout(&our_data.server_created),
             "creating server"
         );
-        assert_eq!(
+        assert!(
-            recv.recv_timeout(wait),
+            try_bool_timeout(&our_data.server_connected),
-            Ok(xwls::StateEvent::ConnectedServer),
             "connecting to server"
         );
 
         let mut f = [pollfd.clone()];
         let start = std::time::Instant::now();
         // Give Xwayland time to do its thing
-        while start.elapsed() < Duration::from_millis(500) {
+
+        let mut ready = our_data.display.lock().unwrap().is_some();
+        while !ready && start.elapsed() < Duration::from_millis(3000) {
             let n = poll(&mut f, 100).unwrap();
             if n > 0 {
                 testwl.dispatch();
             }
+            ready = our_data.display.lock().unwrap().is_some();
         }
 
-        assert_eq!(
+        assert!(ready, "connecting to xwayland");
-            recv.try_recv(),
-            Ok(xwls::StateEvent::XwaylandReady),
-            "connecting to xwayland"
-        );
 
+        let display = our_data.display.lock().unwrap().take().unwrap();
         Self {
             testwl,
             thread: ManuallyDrop::new(thread),
             pollfd,
+            display,
         }
     }
 
@@ -217,6 +269,15 @@ impl Fixture {
     }
 }
 
+xcb::atoms_struct! {
+    struct Atoms {
+        wm_protocols => b"WM_PROTOCOLS",
+        wm_delete_window => b"WM_DELETE_WINDOW",
+        wm_class => b"WM_CLASS",
+        wm_name => b"WM_NAME",
+    }
+}
+
 struct Connection {
     inner: xcb::Connection,
     pollfd: PollFd<'static>,
@@ -231,10 +292,8 @@ impl std::ops::Deref for Connection {
 }
 
 impl Connection {
-    fn new() -> Self {
+    fn new(display: &str) -> Self {
-        // TODO: this will not work if there is an Xserver at 1024, or whenever we add multiple
+        let (inner, _) = xcb::Connection::connect(Some(display)).unwrap();
-        // tests.
-        let (inner, _) = xcb::Connection::connect(Some(":0")).unwrap();
         let fd = unsafe { BorrowedFd::borrow_raw(inner.as_raw_fd()) };
         let pollfd = PollFd::from_borrowed_fd(fd, PollFlags::IN);
         let atoms = Atoms::intern_all(&inner).unwrap();
@@ -258,7 +317,7 @@ impl Connection {
 #[test]
 fn toplevel_flow() {
     let mut f = Fixture::new();
-    let mut connection = Connection::new();
+    let mut connection = Connection::new(&f.display);
     let (window, surface) = f.create_toplevel(&connection.inner, 200, 200);
 
     connection