git.lucas.co / cce-ui
GPU-accelerated UI toolkit (Vulkan)
git clone https://git.lucas.co/cce-ui.git

commitd72a3b97e05498e0016ac7839a854ef3ba7b8d22
parent5b883021d8
authorLucas Galante <lsgalante12@gmail.com>
date2026-09-25 11:30
fix(layout): corner-radius getters no longer nest a style-registry read

Twelve corner-radius getters read their slot as
`get_style_registry().read().unwrap().get_float(k).unwrap_or_else(g)`,
which holds the read guard to the end of the statement, so the fallback
getter `g` (control_/textbox_corner_radius) took a SECOND read on the
same thread. std's RwLock queues readers behind a waiting writer: a
writer arriving between the two reads parks both, and every reader in
the process behind them.

That hung `cargo test -p cce-designer` (2026-09-25): a params-pane paint
parked in spinbox_corner_radius -> control_corner_radius while the
designer's reload_config (one write per config line, on every
State::new) waited for the write lock. Caught with gdb as the test
binary's parent; downstream suites build this crate without cfg(test),
so the per-thread style overlay does not shield them.

The getters now read through registry_float, which releases its guard
before returning. tests/style_registry_reentrancy.rs hammers the write
lock against all twelve in a re-executed child (empty XDG_CONFIG_HOME,
so every fallback fires) and fails on a 30 s deadline rather than
hanging: it deadlocked on the old code and passes in 2 s now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

 src/layout.rs                      |  33 ++++++----
 tests/style_registry_reentrancy.rs | 126 +++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+), 12 deletions(-)

diff --git a/src/layout.rs b/src/layout.rs
index 490dff3..463bb17 100644
--- a/src/layout.rs
+++ b/src/layout.rs
@@ -2185,6 +2185,15 @@ pub fn root_plate_inset() -> f32 {
 
 /// One style-registry float, initialising the registry on first use — the
 /// one read every rung getter goes through.
+///
+/// The guard is released before this returns, which is why a getter whose
+/// unset slot falls back to ANOTHER getter must read through here:
+/// `get_style_registry().read().unwrap().get_float(k).unwrap_or_else(g)`
+/// holds its guard to the end of the statement, so `g`'s read nests inside
+/// it, and std's `RwLock` queues a reader behind a waiting writer — a
+/// `reload_config` arriving between the two reads parks both, and every
+/// reader in the process behind them. That hung cce-designer's suite
+/// (2026-09-25); `tests/style_registry_reentrancy.rs` is the check.
 fn registry_float(slot: &str) -> Option<f32> {
     lazy_init_style_registry();
     get_style_registry().read().unwrap().get_float(slot)
@@ -2239,7 +2248,7 @@ pub fn set_control_relief(relief: bool) {
 
 pub fn toggle_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("toggle_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("toggle_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_toggle_corner_radius(radius: f32) {
@@ -2279,7 +2288,7 @@ pub fn set_toggle_border_width(width: f32) {
 
 pub fn slider_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("slider_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("slider_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 /// The slider band's knobs (`style.control.slider.*`): the flat band's thickness,
@@ -3148,7 +3157,7 @@ pub fn set_button_font(font: &str) {
 pub fn color_selector_preview_corner_radius() -> f32 {
     lazy_init_style_registry();
     // Unset: the swatch rounds like the text field beside it (the TextBox radius).
-    get_style_registry().read().unwrap().get_float("color_selector_preview_corner_radius").unwrap_or_else(textbox_corner_radius)
+    registry_float("color_selector_preview_corner_radius").unwrap_or_else(textbox_corner_radius)
 }
 
 pub fn set_color_selector_preview_corner_radius(radius: f32) {
@@ -3161,7 +3170,7 @@ pub fn set_color_selector_preview_corner_radius(radius: f32) {
 pub fn color_selector_corner_radius() -> f32 {
     lazy_init_style_registry();
     // Unset: the selector's frame rounds like the text field it stands in for.
-    get_style_registry().read().unwrap().get_float("color_selector_corner_radius").unwrap_or_else(textbox_corner_radius)
+    registry_float("color_selector_corner_radius").unwrap_or_else(textbox_corner_radius)
 }
 
 pub fn set_color_selector_corner_radius(radius: f32) {
@@ -3192,7 +3201,7 @@ pub fn set_control_corner_radius(radius: f32) {
 
 pub fn button_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("button_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("button_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_button_corner_radius(radius: f32) {
@@ -3204,7 +3213,7 @@ pub fn set_button_corner_radius(radius: f32) {
 
 pub fn spinbox_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("spinbox_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("spinbox_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_spinbox_corner_radius(radius: f32) {
@@ -3354,7 +3363,7 @@ pub fn set_spinbox_button_padding(padding: f32) {
 
 pub fn textbox_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("textbox_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("textbox_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_textbox_corner_radius(radius: f32) {
@@ -3403,7 +3412,7 @@ pub fn set_textbox_multiline_border_width(width: f32) {
 
 pub fn list_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("list_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("list_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_list_corner_radius(radius: f32) {
@@ -3415,7 +3424,7 @@ pub fn set_list_corner_radius(radius: f32) {
 
 pub fn tree_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("tree_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("tree_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_tree_corner_radius(radius: f32) {
@@ -3601,7 +3610,7 @@ pub fn set_graph_connector_activation_radius(radius: f32) {
 
 pub fn font_selector_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("font_selector_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("font_selector_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_font_selector_corner_radius(radius: f32) {
@@ -3619,12 +3628,12 @@ pub fn set_font_selector_corner_radius(radius: f32) {
 /// its corner belongs to the control scale, like the dropdown's.
 pub fn menu_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("menu_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("menu_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn dropdown_corner_radius() -> f32 {
     lazy_init_style_registry();
-    get_style_registry().read().unwrap().get_float("dropdown_corner_radius").unwrap_or_else(control_corner_radius)
+    registry_float("dropdown_corner_radius").unwrap_or_else(control_corner_radius)
 }
 
 pub fn set_dropdown_corner_radius(radius: f32) {
diff --git a/tests/style_registry_reentrancy.rs b/tests/style_registry_reentrancy.rs
new file mode 100644
index 0000000..16a2c97
--- /dev/null
+++ b/tests/style_registry_reentrancy.rs
@@ -0,0 +1,126 @@
+//! No style getter may read the registry while it already holds a read of it.
+//!
+//! `STYLE_REGISTRY` is a `std::sync::RwLock`, and std's lock queues new readers
+//! behind a WAITING writer. A getter written as
+//!
+//! ```ignore
+//! get_style_registry().read().unwrap().get_float("toggle_corner_radius")
+//!     .unwrap_or_else(control_corner_radius)
+//! ```
+//!
+//! keeps its guard alive to the end of the statement, so the fallback's own
+//! read is a second read on the same thread. A writer that arrives between
+//! the two waits for the first, the second waits for the writer, and every
+//! reader in the process then queues behind both. That hung
+//! `cargo test -p cce-designer` (2026-09-25): ~40 test threads parked in
+//! `RwLock::read_contended` on `STYLE_REGISTRY`, the writer being the
+//! designer's `reload_config` (one write per config line, on every
+//! `State::new`) and the reader any unset corner radius.
+//!
+//! An integration test on purpose: here cce-ui is built WITHOUT `cfg(test)`,
+//! as it is inside every app's suite, so the per-thread style overlay the unit
+//! tests get is out of the way and the getters hit the shared lock.
+//!
+//! The stress runs in a CHILD process — this binary re-executed — because a
+//! deadlock on the process-wide registry cannot be recovered from in-process:
+//! the parent kills the child at a deadline and fails, rather than hanging.
+
+use std::process::{Command, Stdio};
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::Arc;
+use std::time::{Duration, Instant};
+
+use cce_ui::layout;
+
+const CHILD_ENV: &str = "CCE_UI_REENTRANCY_CHILD";
+
+/// Every getter whose unset slot falls back to another style getter — the
+/// shape that nests. With an empty config each of them takes the fallback.
+const FALLBACK_GETTERS: &[fn() -> f32] = &[
+    layout::toggle_corner_radius,
+    layout::slider_corner_radius,
+    layout::color_selector_preview_corner_radius,
+    layout::color_selector_corner_radius,
+    layout::button_corner_radius,
+    layout::spinbox_corner_radius,
+    layout::textbox_corner_radius,
+    layout::list_corner_radius,
+    layout::tree_corner_radius,
+    layout::font_selector_corner_radius,
+    layout::menu_corner_radius,
+    layout::dropdown_corner_radius,
+];
+
+/// The child: one thread takes and drops the write lock as fast as it can,
+/// the others call every fallback getter. Against a nested read this parks
+/// within milliseconds; without one it runs out its time and exits.
+fn hammer() {
+    // Initialise (reload_config runs once) before the race starts, so the
+    // writer is the only writer.
+    for g in FALLBACK_GETTERS {
+        g();
+    }
+    let stop = Arc::new(AtomicBool::new(false));
+    let writer = {
+        let stop = stop.clone();
+        std::thread::spawn(move || {
+            while !stop.load(Ordering::Relaxed) {
+                drop(layout::get_style_registry().write().unwrap());
+            }
+        })
+    };
+    let readers: Vec<_> = (0..4)
+        .map(|_| {
+            std::thread::spawn(|| {
+                let until = Instant::now() + Duration::from_secs(2);
+                while Instant::now() < until {
+                    for g in FALLBACK_GETTERS {
+                        std::hint::black_box(g());
+                    }
+                }
+            })
+        })
+        .collect();
+    for r in readers {
+        r.join().unwrap();
+    }
+    stop.store(true, Ordering::Relaxed);
+    writer.join().unwrap();
+}
+
+#[test]
+fn fallback_getters_never_nest_a_registry_read() {
+    if std::env::var_os(CHILD_ENV).is_some() {
+        hammer();
+        return;
+    }
+
+    // An empty config, so no slot is set and every getter falls back. The
+    // directory is never created; an absent config.kdl reads as no config.
+    let config_home = std::env::temp_dir().join(format!("cce-ui-reentrancy-{}", std::process::id()));
+    let mut child = Command::new(std::env::current_exe().unwrap())
+        .args(["--exact", "fallback_getters_never_nest_a_registry_read", "--test-threads=1", "-q"])
+        .env(CHILD_ENV, "1")
+        .env("XDG_CONFIG_HOME", &config_home)
+        .stdout(Stdio::null())
+        .stderr(Stdio::inherit())
+        .spawn()
+        .expect("re-execute the test binary");
+
+    let deadline = Instant::now() + Duration::from_secs(30);
+    loop {
+        if let Some(status) = child.try_wait().unwrap() {
+            assert!(status.success(), "the stress child failed: {status}");
+            return;
+        }
+        if Instant::now() > deadline {
+            let _ = child.kill();
+            let _ = child.wait();
+            panic!(
+                "style getters deadlocked on STYLE_REGISTRY: a getter read the registry while \
+                 holding a read of it, and a queued writer parked both"
+            );
+        }
+        std::thread::sleep(Duration::from_millis(50));
+    }
+}