Wayland compositor (wlroots)
git clone https://git.lucas.co/cce-compositor.git
wm: fix the status-segment mis-slot wedge and the fd-leak session collapse
After a bar-service restart, a segment (observed: the tray) could park at a
stale position outside the arranged run, surviving ccectl reload and every
later arrange, until its child process was killed. Reproduced headless
(~1/100 restarts): a client whose connection dies mid-startup reconnects and
recreates its toplevel; one interleaving left the final window Mapped but
outside rendering_requested.list, where render_finish never applies the
correct slot the arrange pass keeps computing.
Three layers of fix:
- wl_list_remove now matches upstream libwayland: null the removed node's
pointers and no-op an already-removed node. The port left them stale, so
a re-remove (or any next/is_linked check against them) wrote through
pointers into whatever list the node used to be in — the corruption class
that produced self-looped, heal-invisible segment nodes. wl_list_insert
refuses self-inserts (the reachable end state of a dangling head.prev).
- keep_status_bar_on_top / raise_window drop the stale-prone
`next != head` tail check for an unconditional safe move-to-tail: same
final order, and a Mapped-but-unlinked window is healed every pass
instead of skipped when its stale next happened to equal the head.
- arrange_views applies every Mapped, non-dragged status segment's planned
position directly (box_geom + scene node), so segment slotting no longer
depends on render-list membership at all.
Independently, the restart churn exposed a session-killing leak:
status_server only reaped subscribers on failed writes, and the last_line
dedup means a quiet topic never writes — every bar restart stranded its
whole subscriber set (~10 fds). Enough restarts exhausted RLIMIT_NOFILE,
and both accept loops then spun at 100% CPU flooding the log (167GB
observed). Subscribers are now reaped by read-EOF each loop, accept errors
back off instead of hot-looping, and the fd ceiling rises to 65536
(children still get the original back).
Kept as tools: `ccectl debug-windows` (per-window state/link/configure and
WM-machine dump) and silent-unless-anomalous integrity checks at each
transaction phase (UNLINKED-MAPPED, render-list consistency/reachability).
Validated: 400 consecutive headless bar restarts with SNI churn, zero
mis-slots (baseline ~1/100), fd count flat; 21 lib tests pass.
Co-Authored-By: Claude Fable 5 <[email protected]>
src/server/ipc_server.rs | 5 ++
src/server/process.rs | 7 +-
src/server/server.rs | 20 +++++
src/server/status_server.rs | 41 ++++++++++
src/server/window.rs | 24 ++++++
src/server/window_manager.rs | 179 +++++++++++++++++++++++++++++++++++++++----
src/server/xdg_toplevel.rs | 4 +
7 files changed, 263 insertions(+), 17 deletions(-)
diff --git a/src/server/ipc_server.rs b/src/server/ipc_server.rs
index d8584ea..0943007 100644
--- a/src/server/ipc_server.rs
+++ b/src/server/ipc_server.rs
@@ -53,7 +53,12 @@ fn ipc_server_main(tx: mpsc::Sender<IpcRequest>, display_socket: Option<String>)
});
}
Err(e) => {
+ // EMFILE and friends leave the socket readable, so a bare
+ // continue spins this thread at 100% and floods the log
+ // (167GB observed under fd exhaustion). Back off instead —
+ // the session is degraded but stays diagnosable.
log::error!("[ipc] accept error: {}", e);
+ thread::sleep(std::time::Duration::from_millis(100));
}
}
}
diff --git a/src/server/process.rs b/src/server/process.rs
index 97b7a75..876676e 100644
--- a/src/server/process.rs
+++ b/src/server/process.rs
@@ -22,7 +22,12 @@ pub fn setup() {
rlim_max: max,
});
- let new_cur = std::cmp::min(4096, max);
+ // A compositor's legitimate fd usage scales with clients × buffers
+ // (every imported dmabuf holds one), and hitting the ceiling turns
+ // accept() into an EMFILE spin that takes the session down — 4096
+ // proved reachable under client-reconnect churn. Children get the
+ // original limit back via cleanup_child.
+ let new_cur = std::cmp::min(65536, max);
if let Err(e) = setrlimit(Resource::RLIMIT_NOFILE, new_cur, max) {
log::error!("setrlimit failed: {}, using system default limit of {}", e, cur);
} else {
diff --git a/src/server/server.rs b/src/server/server.rs
index a798a62..c837be2 100644
--- a/src/server/server.rs
+++ b/src/server/server.rs
@@ -211,6 +211,13 @@ pub unsafe fn wl_list_insert(list: *mut WlList, elm: *mut WlList) {
log::error!("wl_list_insert: elm is null!");
return;
}
+ if list == elm {
+ // Inserting a node after itself severs it into a self-loop while
+ // outside pointers may still reference it — always a caller bug
+ // (reachable when a stale head.prev names the node being inserted).
+ log::error!("wl_list_insert: elm == list, refusing self-insert");
+ return;
+ }
(*elm).prev = list;
(*elm).next = (*list).next;
(*(*list).next).prev = elm;
@@ -218,8 +225,21 @@ pub unsafe fn wl_list_insert(list: *mut WlList, elm: *mut WlList) {
}
pub unsafe fn wl_list_remove(elm: *mut WlList) {
+ // Upstream libwayland nulls the removed element's pointers; this port
+ // originally left them stale, so a second remove — or any later
+ // tail/linked check against them — wrote through pointers into whatever
+ // list the node USED to be in, silently corrupting live members. That
+ // corruption class is how a status segment ended up self-looped and
+ // invisible to every re-link heal (the tray parked outside the right
+ // group after bar restarts). Match C semantics: null after unlinking,
+ // and no-op an already-removed node instead of dereferencing null.
+ if (*elm).prev.is_null() || (*elm).next.is_null() {
+ return;
+ }
(*(*elm).next).prev = (*elm).prev;
(*(*elm).prev).next = (*elm).next;
+ (*elm).prev = std::ptr::null_mut();
+ (*elm).next = std::ptr::null_mut();
}
pub unsafe fn wl_list_remove_and_reinit(elm: *mut WlList) {
diff --git a/src/server/status_server.rs b/src/server/status_server.rs
index 94c2615..704d4f6 100644
--- a/src/server/status_server.rs
+++ b/src/server/status_server.rs
@@ -163,12 +163,53 @@ fn status_server_main(rx: mpsc::Receiver<StatusMsg>, display_socket: Option<Stri
break;
}
Err(e) => {
+ // EMFILE and friends: the socket stays readable, so
+ // without a pause this loop (and its log line) spins the
+ // thread at 100% — observed as a 167GB log once dead
+ // subscribers had exhausted the fd table.
log::error!("[status] accept error: {}", e);
+ std::thread::sleep(std::time::Duration::from_millis(100));
break;
}
}
}
+ // Reap dead subscribers by reading: a subscriber never sends after
+ // its subscription line, so a successful zero-byte read is EOF (the
+ // client vanished). Waiting for a WRITE to fail leaked them instead
+ // — last_line dedup means a quiet topic may never write again, and
+ // every bar restart stranded its whole subscriber set. Enough
+ // restarts exhausted the fd table and took the session down.
+ {
+ let mut buf = [0u8; 64];
+ let mut dead_clients = Vec::new();
+ for (i, client) in clients.iter_mut().enumerate() {
+ loop {
+ use std::io::Read;
+ match client.stream.read(&mut buf) {
+ Ok(0) => {
+ log::info!(
+ "[status] client {:?} disconnected (eof)",
+ client.subscription
+ );
+ dead_clients.push(i);
+ break;
+ }
+ // Unexpected chatter: drain and keep the client.
+ Ok(_) => continue,
+ Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => break,
+ Err(_) => {
+ dead_clients.push(i);
+ break;
+ }
+ }
+ }
+ }
+ for i in dead_clients.into_iter().rev() {
+ clients.remove(i);
+ }
+ }
+
// Process incoming updates from the main loop
let mut dismiss_events: Vec<String> = Vec::new();
loop {
diff --git a/src/server/window.rs b/src/server/window.rs
index 13171c6..04f43a6 100644
--- a/src/server/window.rs
+++ b/src/server/window.rs
@@ -1088,6 +1088,10 @@ impl Window {
pub unsafe fn map(&mut self) -> Result<(), &'static str> {
log::debug!("window '{:?}' mapped", self.get_title());
+ if self.get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::debug!("[LinkDbg] map app={:?} was_state={:?} linked={}",
+ self.get_app_id_string(), self.state, self.is_linked());
+ }
assert!(!matches!(self.impl_type, WindowImpl::Destroying));
assert_eq!(self.state, WindowState::Initialized);
self.state = WindowState::Mapped;
@@ -1272,6 +1276,10 @@ impl Window {
pub unsafe fn set_closing(&mut self) {
if self.state != WindowState::Closing {
+ if self.get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::debug!("[LinkDbg] set_closing app={:?} was_state={:?} was_linked={}",
+ self.get_app_id_string(), self.state, self.is_linked());
+ }
self.state = WindowState::Closing;
if self.is_linked() {
wl_list_remove_and_reinit(&mut self.node.link as *mut ffi::wl_list as *mut WlList);
@@ -1281,6 +1289,10 @@ impl Window {
pub unsafe fn unmap(&mut self) {
log::debug!("window '{:?}' unmapped", self.get_title());
+ if self.get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::debug!("[LinkDbg] unmap app={:?} state={:?} linked={}",
+ self.get_app_id_string(), self.state, self.is_linked());
+ }
if self.state != WindowState::Mapped {
return;
}
@@ -1486,6 +1498,10 @@ impl Window {
match self.state {
WindowState::Init => {}
WindowState::Closing => {
+ if self.get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::debug!("[LinkDbg] manage_start closing->init app={:?} was_linked={}",
+ self.get_app_id_string(), self.is_linked());
+ }
self.state = WindowState::Init;
self.wm_sent = WmSentState {
dimensions_hint: DimensionsHint { min_width: 0, min_height: 0, max_width: 0, max_height: 0 },
@@ -1532,6 +1548,10 @@ impl Window {
if wm_v1.is_null() {
let is_linked = self.is_linked();
if !is_linked {
+ if self.get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::debug!("[LinkDbg] manage_start LINK app={:?} state={:?}",
+ self.get_app_id_string(), self.state);
+ }
if !self.node.link.prev.is_null() && !self.node.link.next.is_null() {
wl_list_remove_and_reinit(&mut self.node.link as *mut ffi::wl_list as *mut WlList);
}
@@ -1806,6 +1826,10 @@ impl Window {
if self.wm_requested.dimensions.is_none() && self.wm_requested.fullscreen.is_null() {
return false;
}
+ if self.get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::debug!("[LinkDbg] manage_finish ready->initialized app={:?} linked={}",
+ self.get_app_id_string(), self.is_linked());
+ }
self.state = WindowState::Initialized;
}
WindowState::Initialized | WindowState::Mapped => {}
diff --git a/src/server/window_manager.rs b/src/server/window_manager.rs
index d9f7b06..d9c35d4 100644
--- a/src/server/window_manager.rs
+++ b/src/server/window_manager.rs
@@ -1476,6 +1476,7 @@ impl WindowManager {
let mt_seats = mt0.map(|s| s.elapsed().as_micros());
self.arrange_views();
+ self.debug_check_unlinked_status("manage_start end");
if let (Some(s), Some(a), Some(o), Some(w), Some(t)) =
(mt0, mt_auto, mt_outputs, mt_windows, mt_seats)
@@ -1495,6 +1496,74 @@ impl WindowManager {
}
}
+ /// Wedge tracer: a Mapped status window outside the render list is
+ /// invisible to configures and render_finish — exactly the tray
+ /// mis-slot wedge. Silent unless one exists.
+ unsafe fn debug_check_unlinked_status(&self, phase: &str) {
+ for &w in self.windows.iter() {
+ if w.is_null() || (*w).closed {
+ continue;
+ }
+ if !matches!((*w).state, crate::window::WindowState::Mapped) {
+ continue;
+ }
+ if (*w).is_linked() {
+ continue;
+ }
+ if (*w).get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::info!("[LinkDbg] UNLINKED-MAPPED at {}: app={:?} link.prev_self={} link.prev_null={}",
+ phase,
+ (*w).get_app_id_string(),
+ (*w).node.link.prev == &(*w).node.link as *const ffi::wl_list as *mut ffi::wl_list,
+ (*w).node.link.prev.is_null());
+ }
+ }
+ self.debug_check_render_list(phase);
+ }
+
+ /// Structural check of rendering_requested.list: every member's neighbor
+ /// pointers must agree, and every Mapped status window must be reachable
+ /// from the head. Silent when consistent.
+ unsafe fn debug_check_render_list(&self, phase: &str) {
+ let head = &self.rendering_requested.list as *const ffi::wl_list as *mut WlList;
+ let mut members: Vec<*mut WlList> = Vec::new();
+ let mut curr = (*head).next;
+ let mut steps = 0;
+ while curr != head {
+ if curr.is_null() {
+ log::info!("[LinkDbg] LIST BROKEN at {}: null next after {} steps", phase, steps);
+ return;
+ }
+ if (*(*curr).next).prev != curr {
+ log::info!("[LinkDbg] LIST INCONSISTENT at {}: member {:p} next.prev mismatch", phase, curr);
+ }
+ members.push(curr);
+ curr = (*curr).next;
+ steps += 1;
+ if steps > 10000 {
+ log::info!("[LinkDbg] LIST CYCLE at {}: >10000 members", phase);
+ return;
+ }
+ }
+ for &w in self.windows.iter() {
+ if w.is_null() || (*w).closed {
+ continue;
+ }
+ if !matches!((*w).state, crate::window::WindowState::Mapped) {
+ continue;
+ }
+ if !(*w).get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ continue;
+ }
+ let node = &(*w).node.link as *const ffi::wl_list as *mut WlList;
+ let reachable = members.contains(&node);
+ if (*w).is_linked() && !reachable {
+ log::info!("[LinkDbg] ORPHAN-RING at {}: app={:?} is_linked=true but unreachable from head",
+ phase, (*w).get_app_id_string());
+ }
+ }
+ }
+
pub unsafe fn manage_finish(&mut self) {
assert!(matches!(self.state, WindowManagerState::Manage));
self.cancel_timeout_timer();
@@ -1534,6 +1603,7 @@ impl WindowManager {
if let WindowManagerState::InflightConfigures(count) = self.state {
log::debug!("sent {} tracked configure(s)", count);
+ self.debug_check_unlinked_status("manage_finish end");
if count > 0 {
self.start_timeout_timer(100);
} else {
@@ -1800,6 +1870,7 @@ impl WindowManager {
(*self.server).idle_inhibit_manager.check_active();
log::debug!("finished committing transaction");
+ self.debug_check_unlinked_status("render_finish end");
if self.scheduled.dirty || self.scheduled.dirty_lazy || self.rendering_scheduled.dirty {
self.add_dirty_idle();
@@ -2292,12 +2363,30 @@ impl WindowManager {
}
}
- // Force configure for all status bar windows so they receive the new geometry immediately
+ // Force configure for all status bar windows so they receive the new
+ // geometry immediately, and apply the planned position DIRECTLY.
+ // Positions normally land in render_finish, which only reaches
+ // windows linked into rendering_requested.list — a segment that
+ // dropped out of that list (the reconnect-churn wedge) kept its
+ // stale slot through every later arrange while the plan held the
+ // correct one. The arrange pass is the authority on segment slots,
+ // so make every pass re-slot every segment except one the user is
+ // dragging (the seat op owns its position until release).
for &win_ptr in self.windows.iter() {
if !win_ptr.is_null() && !(*win_ptr).closed && (*win_ptr).is_status_bar() {
if (*win_ptr).wm_requested.dimensions.is_some() {
(*win_ptr).manage_finish();
}
+ if matches!((*win_ptr).state, crate::window::WindowState::Mapped)
+ && !self.is_window_being_moved(win_ptr)
+ {
+ let x = (*win_ptr).rendering_requested.x;
+ let y = (*win_ptr).rendering_requested.y;
+ (*win_ptr).box_geom.x = x;
+ (*win_ptr).box_geom.y = y;
+ ffi::river_scene_node_set_position_if_changed((*win_ptr).tree as *mut ffi::wlr_scene_node, x, y);
+ ffi::river_scene_node_set_position_if_changed((*win_ptr).popup_tree as *mut ffi::wlr_scene_node, x, y);
+ }
}
}
@@ -2782,15 +2871,25 @@ impl WindowManager {
let node_link = &mut (*win_ptr).node.link as *mut ffi::wl_list as *mut WlList;
let list_head = &mut self.rendering_requested.list as *mut ffi::wl_list as *mut WlList;
if !node_link.is_null() && !list_head.is_null() {
- if (*node_link).next != list_head {
- if (*win_ptr).is_linked() {
- crate::server::wl_list_remove_and_reinit(node_link);
- }
- let last = (*list_head).prev;
- if !last.is_null() {
- crate::server::wl_list_insert(last, node_link);
- }
+ // Unconditional remove+reinsert, not a `next != head` tail
+ // check: a node with a stale next that happened to equal the
+ // head skipped the move here AND read as linked to
+ // manage_start, so nothing ever re-attached it — the segment
+ // froze at its last applied position (the tray mis-slot
+ // wedge). The final order is identical (each Mapped status
+ // window moves to the tail in windows order) and the
+ // primitives no-op cleanly on every unlinked pointer state.
+ if !(*win_ptr).is_linked() {
+ log::info!("[LinkDbg] keep_on_top healing unlinked app={:?}",
+ (*win_ptr).get_app_id_string());
}
+ crate::server::wl_list_remove_and_reinit(node_link);
+ let last = (*list_head).prev;
+ // head.prev can only name this node via pre-existing
+ // corruption (a dangling backpointer); fall back to the head
+ // so the node still rejoins the list.
+ let after = if last.is_null() || last == node_link { list_head } else { last };
+ crate::server::wl_list_insert(after, node_link);
}
}
}
@@ -2802,14 +2901,16 @@ impl WindowManager {
let node_link = &mut (*window).node.link as *mut ffi::wl_list as *mut WlList;
let list_head = &mut self.rendering_requested.list as *mut ffi::wl_list as *mut WlList;
if !node_link.is_null() && !list_head.is_null() {
- if (*node_link).next != list_head {
- if (*window).is_linked() {
- crate::server::wl_list_remove_and_reinit(node_link);
- }
+ // Same shape as keep_status_bar_on_top: no `next != head` tail
+ // check (stale pointers made it lie), just a safe move-to-tail.
+ // A window that isn't Mapped stays out of the render list — its
+ // linking is manage_start's job, and force-inserting a
+ // Closing/Init window here would resurrect it for one frame.
+ if (*window).is_linked() || matches!((*window).state, crate::window::WindowState::Mapped) {
+ crate::server::wl_list_remove_and_reinit(node_link);
let last = (*list_head).prev;
- if !last.is_null() {
- crate::server::wl_list_insert(last, node_link);
- }
+ let after = if last.is_null() || last == node_link { list_head } else { last };
+ crate::server::wl_list_insert(after, node_link);
}
}
self.keep_status_bar_on_top();
@@ -3067,6 +3168,52 @@ impl WindowManager {
}
return out;
}
+ // WM introspection: one line per window with the fields the
+ // arrange pass keys on (state, render-list linkage, status edge,
+ // seat-op move, requested vs applied position, configure state).
+ // Found the tray segment mis-slot wedge; kept as a debugging tool.
+ "debug-windows" => {
+ let mut out = format!(
+ "wm state={:?} dirty={} dirty_lazy={} rendering_dirty={} dirty_idle_armed={} wm_object={}\n",
+ self.state,
+ self.scheduled.dirty,
+ self.scheduled.dirty_lazy,
+ self.rendering_scheduled.dirty,
+ !self.dirty_idle.is_null(),
+ !self.object.is_null(),
+ );
+ for &w in self.windows.iter() {
+ if w.is_null() {
+ continue;
+ }
+ let cfg = match (*w).impl_type {
+ crate::window::WindowImpl::Toplevel(t) if !t.is_null() => {
+ format!("{:?}", (*t).configure_state)
+ }
+ _ => "-".to_string(),
+ };
+ out.push_str(&format!(
+ "window id={} app_id={:?} state={:?} closed={} linked={} mode={:?} edge={:?} moved={} req_pos=({},{}) box=({},{},{}x{}) collapsed_len={} cfg={}\n",
+ (*w).ref_key.index,
+ (*w).get_app_id_string().unwrap_or_default(),
+ (*w).state,
+ (*w).closed,
+ (*w).is_linked(),
+ (*w).tiling_mode,
+ (*w).status_edge,
+ self.is_window_being_moved(w),
+ (*w).rendering_requested.x,
+ (*w).rendering_requested.y,
+ (*w).box_geom.x,
+ (*w).box_geom.y,
+ (*w).box_geom.width,
+ (*w).box_geom.height,
+ (*w).status_collapsed_len,
+ cfg,
+ ));
+ }
+ return out;
+ }
"status-hide-mode" => {
let enable = if parts.len() >= 2 {
match parts[1] {
diff --git a/src/server/xdg_toplevel.rs b/src/server/xdg_toplevel.rs
index 9dabf85..7ece495 100644
--- a/src/server/xdg_toplevel.rs
+++ b/src/server/xdg_toplevel.rs
@@ -673,6 +673,10 @@ unsafe extern "C" fn handle_commit(listener: *mut ffi::wl_listener, _data: *mut
if ffi::river_wlr_xdg_surface_get_initial_commit(base) {
assert!((*window).state != crate::window::WindowState::Ready);
+ if (*window).get_app_id_string().map_or(false, |id| id.starts_with("cce-status")) {
+ log::debug!("[LinkDbg] initial commit -> ready app={:?} was_state={:?} linked={}",
+ (*window).get_app_id_string(), (*window).state, (*window).is_linked());
+ }
(*window).state = crate::window::WindowState::Ready;
let mut new_geometry = std::mem::zeroed();
ffi::river_wlr_xdg_surface_get_geometry(base, &mut new_geometry);