From 53c8eefa9972270483bd8c701ec166c24ef62e6b Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 6 Jul 2026 23:58:53 +0200 Subject: [PATCH] =?UTF-8?q?fix(client-linux):=20break=20the=20FlowBox=20ac?= =?UTF-8?q?tivation=20signal=20cycle=20=E2=80=94=20stack=20overflow=20on?= =?UTF-8?q?=20every=20host-card=20click?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `child-activated` (fired by a pointer click) was bridged to `child.activate()` so each card's own connect handler (wired on the child's `activate` signal) would run. But `child.activate()` runs `GtkFlowBoxChild`'s default handler, which re-emits `child-activated` on the FlowBox — bouncing straight back into the same closure. Unguarded, that ping-pong recursed forever, overflowing the stack on every single host-card click or Enter-key activation (confirmed live via coredump/gdb: 43k+ stack frames of gobject signal emission, and the `fatal runtime error: stack overflow, aborting` in the crash log). A re-entrancy flag breaks the cycle after the one real activation. Added a regression test that wires the identical FlowBox/FlowBoxChild signal cycle against a real display and asserts it returns instead of recursing — it reproduces the exact stack overflow against the old code. Co-Authored-By: Claude Sonnet 5 --- clients/linux/src/ui_hosts.rs | 59 ++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/clients/linux/src/ui_hosts.rs b/clients/linux/src/ui_hosts.rs index fc7d8c85..f72bf9fb 100644 --- a/clients/linux/src/ui_hosts.rs +++ b/clients/linux/src/ui_hosts.rs @@ -162,9 +162,20 @@ pub fn new(settings: Rc>, cbs: HostsCallbacks) -> HostsUi { // A pointer click (and keyboard activate) emits `child-activated` on the *FlowBox*, never // the child's own `activate` signal — so bridge it back to the child, where each card wires // its connect handler (`saved_card`/`discovered_card`). Without this, clicking a card is dead. + // + // `child.activate()` in turn runs `GtkFlowBoxChild`'s own default handler, which re-emits + // `child-activated` on the FlowBox — bouncing straight back into this closure. Unguarded, + // that ping-pong recurses forever and overflows the stack on every single card click/Enter + // (a real crash seen live, not hypothetical); the re-entrancy flag breaks the cycle after + // the one real activation. for flow in [&saved_flow, &disc_flow] { - flow.connect_child_activated(|_, child| { + let activating = std::cell::Cell::new(false); + flow.connect_child_activated(move |_, child| { + if activating.replace(true) { + return; + } child.activate(); + activating.set(false); }); } @@ -720,3 +731,49 @@ fn add_host_dialog(state: &Rc) { } dialog.present(Some(&state.stack)); } + +#[cfg(test)] +mod tests { + use adw::prelude::*; + use std::cell::Cell; + use std::rc::Rc; + + // Reproduces the exact FlowBox/FlowBoxChild wiring from `new()`: `child-activated` bridges + // to `child.activate()`, whose own default handler re-emits `child-activated` on the + // FlowBox — that ping-pong recursed forever (stack overflow on every host-card click/Enter) + // until the re-entrancy guard was added. This exercises the *real* GTK signal cycle, not a + // simulation of it, so it fails the same way the shipped bug did if the guard regresses. + #[test] + #[ignore = "needs a Wayland/X display"] + fn flow_box_activation_bridge_does_not_recurse() { + assert!(gtk::init().is_ok(), "no display"); + + let flow = gtk::FlowBox::builder() + .selection_mode(gtk::SelectionMode::None) + .activate_on_single_click(true) + .build(); + let activating = Cell::new(false); + flow.connect_child_activated(move |_, child| { + if activating.replace(true) { + return; + } + child.activate(); + activating.set(false); + }); + + let child = gtk::FlowBoxChild::new(); + flow.insert(&child, -1); + let fired = Rc::new(Cell::new(0u32)); + { + let fired = fired.clone(); + child.connect_activate(move |_| fired.set(fired.get() + 1)); + } + + // What a pointer click with `activate_on_single_click` does internally: emit + // `child-activated` directly on the FlowBox. A regression here overflows the stack + // instead of returning. + flow.emit_by_name::<()>("child-activated", &[&child]); + + assert_eq!(fired.get(), 1, "the per-card handler should fire exactly once"); + } +}