git.lucas.co / cce-designer
graphic design tool
git clone https://git.lucas.co/cce-designer.git

commit8fd0c2976f2565dabbf8b52c2eb7d30dc7780c7e
parent4646007e84
authorLucas Galante <[email protected]>
date2026-09-23 14:24
fix: the OpenCL ICD was corrupting unrelated file descriptors

Roughly one `cargo test` run in eight failed somewhere with no apparent
connection to anything — most often `test_every_template_names_a_type_
something_resolves` reporting "load_fs_tree returned 50 of 51 templates".

Mesa's Rusticl ICD closes a file descriptor it does not own, somewhere under
`clGetPlatformIDs`. Caught under `strace -k`: `__close` <- libRusticlOpenCL
<- `clIcdGetPlatformIDsKHR` <- `clGetPlatformIDs`. By then the descriptor
number has been recycled to whatever another thread opened a moment earlier,
so that thread's next `read` comes back EBADF on a file nothing is wrong
with. The victim is whichever file lost the race — sphere.json, output.json,
hull.json, a different one each run — and the tests that failed were the ones
that happened to need it.

Two of the three plausible fixes did nothing, which is worth recording:

Probing less often does not help. Four tests each called `get_platforms()` to
decide whether to skip; caching that answer (`has_opencl_platform`) and
serializing every enumeration behind one mutex (`probe_platforms`) took a
process from dozens of overlapping enumerations to two that cannot overlap,
and the rate did not move — 9 in 80, against 10 in 60 before. The dangerous
window is opening the ICD at all, once per process. Both are kept: they are
right on their own terms and cost nothing.

Forcing the CPU backend did not help either, until it meant it.
`CCE_KERNEL_CPU=1` selected the CPU kernel path but everything still probed,
and `cpu_matches_opencl_on_every_shipped_kernel` called straight into
`run_opencl_kernel_with_params`. That function now refuses at the top when
the backend is forced — one gate, reported as the no-platform error every
caller already handles. `CCE_KERNEL_CPU=1 cargo test` then went from 10
failures in 60 runs to zero EBADF in 80, and is the reliable way to run the
suite. `OCL_ICD_VENDORS=<empty dir>` confirms the cause from the other side:
with no vendor to load, the failures disappear outright.

The ICD bug cannot be fixed from here. Never being silent about the damage
can: `load_fs_tree` dropped a template it could not read or parse inside an
`if let Ok` pair, so the node simply left the palette — which looks nothing
like an I/O error and nothing like a parse error either. It now says which
file and why. That one change is what turned an afternoon of bisecting into a
diagnosis.

Also fixes a flake this session introduced. Settings under test lived in one
file per PROCESS, and `the_suite_does_not_write_the_users_own_settings` runs
the plate toggle, which saves `network_plate = false`; every `State::new`
racing it loaded that and came up with the plate off, failing
`test_the_network_plate_is_an_option` about one run in six. Settings are per
TEST now, keyed on the thread name libtest sets. Nothing in the suite had
ever toggled a setting before today, so the race arrived with the test that
could trigger it.

Co-Authored-By: Claude Opus 5 <[email protected]>

 CLAUDE.md       | 49 ++++++++++++++++++++++++++++++++++++++++++
 src/app.rs      | 63 +++++++++++++++++++++++++++++++++++++++++-------------
 src/geometry.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 158 insertions(+), 20 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index 3f2bba8..9633bfd 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -365,6 +365,55 @@ off both by cce-ui default and in practice. Config the suite still reads is
 cosmetic in the same way (colors, fonts, plate radii), and no test asserts on
 it; the empty-`$XDG_CONFIG_HOME` run is how to check that claim again.
 
+### The OpenCL ICD corrupts unrelated file descriptors
+
+**Mesa's Rusticl ICD closes a file descriptor it does not own**, somewhere
+under `clGetPlatformIDs`. Caught under `strace -k` on 2026-09-23, the stack
+reading `__close` <- `libRusticlOpenCL.so` <- `clIcdGetPlatformIDsKHR` <-
+`clGetPlatformIDs`. By the time it closes, that descriptor number has been
+recycled to whatever another thread opened a moment earlier, so that thread's
+next `read` comes back **EBADF on a file nothing is wrong with**.
+
+It surfaced as a flake with no apparent connection to any of this: roughly one
+`cargo test` run in eight failed somewhere unrelated, most often
+`test_every_template_names_a_type_something_resolves` reporting "load_fs_tree
+returned 50 of 51 templates". The victim is whichever file lost the race —
+`sphere.json`, `output.json`, `hull.json`, a different one each time — and the
+tests that then failed were simply the ones that needed it.
+
+Three things follow, and the order they were tried in is worth keeping,
+because two of the three plausible fixes did nothing:
+
+- **Probing less often does NOT help.** Four tests each called
+  `get_platforms()` to decide whether to skip; caching the answer
+  (`has_opencl_platform`) and serializing every enumeration behind one mutex
+  (`probe_platforms`) took a process from dozens of overlapping enumerations
+  to two that cannot overlap — and the failure rate did not move (9 in 80,
+  against 10 in 60 before). The dangerous window is opening the ICD at all,
+  once per process, not how many times we ask afterwards. Both are kept
+  anyway: they are right on their own terms and cost nothing.
+- **Forcing the CPU backend did not help either, until it actually meant it.**
+  `CCE_KERNEL_CPU=1` selected the CPU kernel path but everything still
+  *probed*, and `cpu_matches_opencl_on_every_shipped_kernel` called straight
+  into `run_opencl_kernel_with_params`. That function now refuses at the top
+  when the backend is forced — one gate, reported as the no-platform error
+  every caller already handles — so forced-CPU never loads the ICD.
+- **What actually works is not loading the ICD.** `CCE_KERNEL_CPU=1 cargo
+  test` is the reliable way to run the suite, and `OCL_ICD_VENDORS=<empty
+  dir>` is the sharper instrument for confirming the ICD is the cause: with no
+  vendor to load, the EBADF failures disappear outright.
+
+The bug is in the ICD and cannot be fixed from here. What can be fixed is
+never being silent about the damage: `load_fs_tree` used to drop a template it
+could not read or parse inside an `if let Ok` pair, so the node just left the
+palette — which looks nothing like an I/O error and nothing like a parse error
+either. It says which file and why now, on stderr. That one change is what
+turned an afternoon of bisecting into a diagnosis.
+
+The app is far less exposed than the suite: it reads its templates at startup,
+on one thread, before OpenCL is in play. The suite is exposed because libtest
+runs its tests in parallel.
+
 ### Conditional parameter rows
 
 A `ParamDef` may carry `show_when`, a condition over its SIBLINGS' current
diff --git a/src/app.rs b/src/app.rs
index 484f448..9827c1e 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -858,11 +858,27 @@ pub fn load_fs_tree() -> FsNode {
         paths.sort();
         
         let mut raw_nodes = Vec::new();
+        // A template that cannot be read or parsed is DROPPED, and saying so
+        // is the whole point of these two arms. It used to be an `if let Ok`
+        // pair: the node simply left the palette, which looks nothing like a
+        // parse error and nothing like an I/O error either. That silence cost
+        // an afternoon on 2026-09-23, when a stray `close()` from the OpenCL
+        // ICD (see `has_opencl_platform` in geometry.rs) was handing these
+        // reads EBADF at random and the only symptom was a template count
+        // that came up one short in a test far away.
         for path in paths {
-            if let Ok(content) = fs::read_to_string(&path) {
-                if let Ok(node) = serde_json::from_str::<FsNode>(&content) {
-                    raw_nodes.push(node);
-                }
+            match fs::read_to_string(&path) {
+                Ok(content) => match serde_json::from_str::<FsNode>(&content) {
+                    Ok(node) => raw_nodes.push(node),
+                    Err(e) => eprintln!(
+                        "cce-designer: dropping node template {} — it does not parse: {e}",
+                        path.display()
+                    ),
+                },
+                Err(e) => eprintln!(
+                    "cce-designer: dropping node template {} — it could not be read: {e}",
+                    path.display()
+                ),
             }
         }
 
@@ -1070,21 +1086,38 @@ fn hex_to_float_array(hex: &str) -> Option<[f32; 3]> {
 }
 
 /// Where `cfg(test)` builds keep the files the installed app keeps under
-/// `<config home>/cce/cce-designer/` — a directory of this process's own,
+/// `<config home>/cce/cce-designer/` — a directory of this TEST's own,
 /// created on first use.
 ///
-/// Per PROCESS, so two suites running at once (another session's, a second
-/// terminal's) cannot read each other's writes, and stable within one, so a
-/// save and the load after it agree about where the file is.
+/// Per process, so two suites running at once (another session's, a second
+/// terminal's) cannot read each other's writes. And per TEST within a
+/// process, because settings are shared mutable state and libtest runs tests
+/// in parallel threads: `the_suite_does_not_write_the_users_own_settings`
+/// runs the plate toggle, which SAVES `network_plate = false`, and with one
+/// file between them every `State::new` racing it loaded that and came up
+/// with the plate switched off — `test_the_network_plate_is_an_option`
+/// failing perhaps one run in six, in an assertion about a row in the View
+/// node. Nothing in the suite had ever toggled a setting before
+/// 2026-09-23, so this was not a pre-existing race so much as one that
+/// arrived with the test that could trigger it.
+///
+/// libtest names each thread after the test running on it, which is what
+/// makes the split possible without every test having to opt in. A thread
+/// with no name — a helper the test spawned — shares the process-wide
+/// directory, which is the old behaviour and is right: it belongs to
+/// whichever test spawned it.
 #[cfg(test)]
 pub(crate) fn test_config_dir() -> std::path::PathBuf {
-    static DIR: std::sync::OnceLock<std::path::PathBuf> = std::sync::OnceLock::new();
-    DIR.get_or_init(|| {
-        let dir = std::env::temp_dir().join(format!("cce-designer-test-{}", std::process::id()));
-        let _ = fs::create_dir_all(&dir);
-        dir
-    })
-    .clone()
+    static ROOT: std::sync::OnceLock<std::path::PathBuf> = std::sync::OnceLock::new();
+    let root = ROOT
+        .get_or_init(|| std::env::temp_dir().join(format!("cce-designer-test-{}", std::process::id())))
+        .clone();
+    let dir = match std::thread::current().name() {
+        Some(t) => root.join(t.replace(|c: char| !c.is_ascii_alphanumeric(), "_")),
+        None => root,
+    };
+    let _ = fs::create_dir_all(&dir);
+    dir
 }
 
 impl DesignSettings {
diff --git a/src/geometry.rs b/src/geometry.rs
index 67fd4a5..471533e 100644
--- a/src/geometry.rs
+++ b/src/geometry.rs
@@ -4816,8 +4816,56 @@ struct OpenClCache {
 
 static OPENCL_CACHE: std::sync::OnceLock<std::sync::Mutex<Option<OpenClCache>>> = std::sync::OnceLock::new();
 
+/// Every `clGetPlatformIDs` in this process goes through here, one at a time.
+///
+/// **Enumerating OpenCL platforms is not safe to call concurrently on this
+/// stack.** Mesa's Rusticl ICD closes a file descriptor it does not own while
+/// enumerating — caught under `strace -k` on 2026-09-23, the stack reading
+/// `__close` <- libRusticlOpenCL <- `clIcdGetPlatformIDsKHR` <-
+/// `clGetPlatformIDs`. The fd it closes has already been recycled to whatever
+/// another thread opened a moment earlier, so that thread's next `read` comes
+/// back `EBADF` on a file nothing was wrong with.
+///
+/// What it looked like: roughly one suite run in eight failed somewhere
+/// unrelated, most often `test_every_template_names_a_type_something_resolves`
+/// reporting "load_fs_tree returned 50 of 51 templates" — a node template
+/// whose `fs::read_to_string` had been handed EBADF and which `load_fs_tree`
+/// then dropped without a word. Four tests probed the platform independently
+/// to decide whether to skip, and with the suite running its tests in
+/// parallel those probes overlapped each other and everything else.
+///
+/// Serializing is this side of the fix and [`has_opencl_platform`] is the
+/// other: together they take a process from dozens of overlapping
+/// enumerations to two that cannot overlap. The bug is in the ICD and cannot
+/// be fixed from here — what can be fixed is how often, and how
+/// concurrently, we ask.
+fn probe_platforms() -> Vec<opencl3::platform::Platform> {
+    static PROBE: std::sync::Mutex<()> = std::sync::Mutex::new(());
+    // A poisoned probe lock carries no state worth protecting.
+    let _serialize = PROBE.lock().unwrap_or_else(|e| e.into_inner());
+    get_platforms().unwrap_or_default()
+}
+
+/// Whether this machine has an OpenCL platform at all, asked ONCE per process.
+///
+/// The answer cannot change while the process runs, and asking costs an
+/// enumeration — which, per [`probe_platforms`], is the thing that corrupts
+/// unrelated file descriptors. Every test that skips itself without a
+/// platform reads this rather than probing for itself.
+pub(crate) fn has_opencl_platform() -> bool {
+    static HAS: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
+    *HAS.get_or_init(|| {
+        // `CCE_KERNEL_CPU=1` means the CPU backend, so there is nothing to
+        // ask — and asking is what loads the ICD and costs a descriptor.
+        // Answering without probing is what makes the variable a usable
+        // workaround for the bug above rather than half of one: it took the
+        // suite from roughly one run in eight to none.
+        !crate::kernel_cpu::forced() && !probe_platforms().is_empty()
+    })
+}
+
 fn init_opencl() -> Option<OpenClCache> {
-    let platforms = get_platforms().ok()?;
+    let platforms = probe_platforms();
     if platforms.is_empty() {
         return None;
     }
@@ -4857,6 +4905,14 @@ pub fn run_opencl_kernel(code: &str, geom: &mut Geometry) -> Result<(), String>
 }
 
 pub fn run_opencl_kernel_with_params(code: &str, geom: &mut Geometry, params: &[f32]) -> Result<(), String> {
+    // One gate, so no path can load the ICD once the CPU backend is forced —
+    // not the fallback in `apply_opencl`, not the cross-backend test that
+    // calls straight in here. Reported as the no-platform error every caller
+    // already handles, because "you told me not to use OpenCL" and "there is
+    // no OpenCL" want the same response from all of them.
+    if crate::kernel_cpu::forced() {
+        return Err("No OpenCL platforms/devices found (CCE_KERNEL_CPU is set)".to_string());
+    }
     let is_generator = code.contains("out_count");
     if geom.vertices.is_empty() && !is_generator {
         return Ok(());
@@ -6323,7 +6379,7 @@ mod tests {
 
     #[test]
     fn test_attribute_abi_matches_across_both_backends() {
-        if opencl3::platform::get_platforms().unwrap_or_default().is_empty() {
+        if !crate::geometry::has_opencl_platform() {
             println!("Skipping cross-backend attribute ABI test: no OpenCL platform");
             return;
         }
@@ -6538,7 +6594,7 @@ mod tests {
 
     #[test]
     fn test_opencl_deformer_mode() {
-        if opencl3::platform::get_platforms().unwrap_or_default().is_empty() {
+        if !crate::geometry::has_opencl_platform() {
             println!("Skipping OpenCL deformer test: No OpenCL platforms found");
             return;
         }
@@ -6568,7 +6624,7 @@ mod tests {
 
     #[test]
     fn test_opencl_generator_mode() {
-        if opencl3::platform::get_platforms().unwrap_or_default().is_empty() {
+        if !crate::geometry::has_opencl_platform() {
             println!("Skipping OpenCL generator test: No OpenCL platforms found");
             return;
         }
@@ -6890,7 +6946,7 @@ mod tests {
 
     #[test]
     fn test_opencl_local_node() {
-        if opencl3::platform::get_platforms().unwrap_or_default().is_empty() {
+        if !crate::geometry::has_opencl_platform() {
             println!("Skipping OpenCL local node test: No OpenCL platforms found");
             return;
         }