collective
Browse / github.com/storytold/lightcraft / patch
Issue #3071: rayon's global pool and the ad-hoc worker threads have names
Rayon's global pool and ad-hoc spawned threads have no names, so thread dumps of a stuck run point at the wrong pool Part of #3057. ## Summary LightCraft names its own pools (`lc-job-N`, `lc-demo-scene-N`, `denoise-N`, `lightcraft-faces-…`). Rayon's global pool and some ad-hoc `std::thread::spawn` calls have no name. On Linux an unnamed thread takes the name of the thread that created it, so a thread dump of a stuck or overloaded run points at the wrong pool. In the 137-thread ui-egui test process from #3057, 31 threads were named `lc-job-5`. That fits the 16 global-pool threads having been started from a render worker, plus 14 real `lc-job-5` workers. I worked this out from the counts and didn't check it
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- THEtheChad (imported from pull request #3126, open)
- Base
c9860ea7a3fa367451acd9d8cf23538c345192c6- Size
- +128 −9 in 8 files
- Review
- pending
- Shared
- 2026-10-28
- Id
f9205e696b9140be0790b405
How it went
No other operator has reported using it yet.
Diff
raw · apps/lightcraft-cli/src/main.rs apps/lightcraft/src/control_server.rs apps/lightcraft/src/main.rs crates/engine/src/lib.rs crates/engine/src/threads.rs crates/ui-egui/src/dialogs/import/scan.rs crates/ui-egui/src/panels/left/sections.rs crates/ui-egui/src/sync.rs
@@ -155,6 +155,8 @@ fn main() -> ExitCode { // Warnings (a GPU render redone on the CPU, an unknown backend name) on stderr; LIGHTCRAFT_LOG // or RUST_LOG picks another level (#168). lightcraft_engine::logging::install("lightcraft-cli");+ // before anything uses rayon, so its threads show as `lc-rayon-N` in a thread dump (#3071)+ lightcraft_engine::threads::name_global_pool(); let args: Vec<String> = std::env::args().skip(1).collect(); let r = match args.first().map(String::as_str) { Some("run") => run(&args[1..]),@@ -32,8 +32,11 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver<ControlRequest> { return rx; } };+ if let Err(e) = std::thread::Builder::new().name("lc-control".into()).spawn(move || accept_loop(listener, tx, ctx)) {+ log::error!("control server could not start: {e}");+ return rx;+ } log::info!("control server listening on 127.0.0.1:{port}");- std::thread::spawn(move || accept_loop(listener, tx, ctx)); rx } @@ -57,10 +60,19 @@ fn spawn_connection( let _ = writeln!(stream, "{}", json!({"ok": false, "error": "too many control connections"})); return None; }- Some(std::thread::spawn(move || {+ let slot = Arc::clone(&open);+ match std::thread::Builder::new().name("lc-control-conn".into()).spawn(move || { serve(stream, &tx, &ctx, idle_timeout);- open.fetch_sub(1, Ordering::SeqCst);- }))+ slot.fetch_sub(1, Ordering::SeqCst);+ }) {+ Ok(worker) => Some(worker),+ Err(e) => {+ // (the connection, moved into the thread that didn't start, is closed)+ log::warn!("control connection not served: {e}");+ open.fetch_sub(1, Ordering::SeqCst);+ None+ }+ } } /// Five minutes allows pauses between automation commands without retaining abandoned slots forever.@@ -273,6 +285,15 @@ mod tests { } } + /// A connection's thread is named, so a thread dump tells it from the app's other threads (#3071).+ #[test]+ fn connection_threads_are_named() {+ let open = Arc::new(AtomicUsize::new(0));+ let connection = Connection::new(&open, Duration::from_secs(1), |c| c.shutdown(std::net::Shutdown::Write).unwrap());+ let name = connection.worker.as_ref().and_then(|w| w.thread().name().map(String::from));+ assert_eq!(name.as_deref(), Some("lc-control-conn"));+ }+ #[test] fn idle_connections_release_their_slots_without_dispatching() { for input in ["", "{\"method\":\"engine.execute\""] {@@ -145,6 +145,8 @@ fn main() -> eframe::Result { if let Some(logger) = logger { logging::record_panics(logger); }+ // before anything uses rayon, so its threads show as `lc-rayon-N` in a thread dump (#3071)+ lightcraft_engine::threads::name_global_pool(); alloc_release::install(); let mut control_port = control_port_from("LIGHTCRAFT_CONTROL_PORT", std::env::var("LIGHTCRAFT_CONTROL_PORT").ok().filter(|v| !v.is_empty())); let mut files = Vec::new();@@ -80,6 +80,7 @@ pub mod slideshow_export; pub mod smart; pub mod sync; pub mod text;+pub mod threads; mod view; pub mod walk; new file mode 100644@@ -0,0 +1,89 @@+//! Names for the threads of rayon's global pool (#3071). On Linux an unnamed thread takes the+//! name of the thread that started it, so without this a thread dump of a stuck run shows the+//! global pool's threads under the name of whichever render worker first used rayon.++/// The name of the global pool's thread `i` (Linux keeps the first 15 bytes of a name).+pub fn rayon_thread_name(i: usize) -> String {+ format!("lc-rayon-{i}")+}++/// Build rayon's global pool with its threads named [`rayon_thread_name`], at rayon's default+/// size. The app binaries call this once at start-up, before anything uses rayon; test binaries+/// keep rayon's lazy, unnamed pool. Returns whether the pool was built here: when it already+/// exists, or its threads can't start, it is left as it is (logged at debug level), never a+/// panic.+pub fn name_global_pool() -> bool {+ match rayon::ThreadPoolBuilder::new().thread_name(rayon_thread_name).build_global() {+ Ok(()) => true,+ Err(e) => {+ log::debug!("rayon's global pool keeps its thread names: {e}");+ false+ }+ }+}++#[cfg(test)]+mod tests {+ use super::*;++ #[test]+ fn names_fit_linux_thread_names() {+ assert_eq!(rayon_thread_name(0), "lc-rayon-0");+ assert!(rayon_thread_name(99_999).len() <= 15, "Linux would cut the index off");+ }++ /// `name_global_pool` in a process of its own (the global pool is built once per process):+ /// its threads carry the name, and a second call leaves the pool as it is.+ #[test]+ fn the_global_pool_threads_are_named() {+ use std::io::Read;+ use std::process::Stdio;+ let mut child = std::process::Command::new(std::env::current_exe().unwrap())+ .args(["--exact", "threads::tests::in_a_fresh_process", "--ignored", "--test-threads=1"])+ .env("LC_NAME_GLOBAL_POOL", "1")+ .stdout(Stdio::piped())+ .stderr(Stdio::piped())+ .spawn()+ .unwrap();+ // Drain both pipes on threads so a chatty child can't block on a full pipe.+ let drain = |mut r: Box<dyn Read + Send>| {+ std::thread::spawn(move || {+ let mut s = String::new();+ let _ = r.read_to_string(&mut s);+ s+ })+ };+ let (stdout, stderr) = (drain(Box::new(child.stdout.take().unwrap())), drain(Box::new(child.stderr.take().unwrap())));+ // A stalled child (pool start-up, `broadcast`) fails the test instead of hanging the suite.+ let limit = std::time::Duration::from_secs(60);+ let deadline = std::time::Instant::now() + limit;+ let status = loop {+ if let Some(status) = child.try_wait().unwrap() {+ break status;+ }+ if std::time::Instant::now() > deadline {+ let _ = child.kill();+ let _ = child.wait();+ panic!("the child process did not finish within {limit:?}");+ }+ std::thread::sleep(std::time::Duration::from_millis(20));+ };+ let (stdout, stderr) = (stdout.join().unwrap(), stderr.join().unwrap());+ assert!(status.success() && stdout.contains("1 passed"), "{stdout}\n{stderr}");+ }++ #[test]+ #[ignore = "run in a process of its own by the_global_pool_threads_are_named"]+ fn in_a_fresh_process() {+ if std::env::var_os("LC_NAME_GLOBAL_POOL").is_none() {+ return;+ }+ assert!(name_global_pool(), "the first call builds the pool");+ let names: Vec<Option<String>> = rayon::broadcast(|_| std::thread::current().name().map(String::from));+ assert_eq!(names.len(), rayon::current_num_threads());+ for (i, name) in names.iter().enumerate() {+ assert_eq!(name.as_deref(), Some(rayon_thread_name(i).as_str()));+ }+ assert!(!name_global_pool(), "a second call leaves the pool as it is");+ }+}@@ -69,7 +69,7 @@ pub fn open(app: &mut LightcraftApp, paths: Vec<String>) -> Result<Value, String let _ = tx.send(scan_with(input, &paths, &p)); }; #[cfg(not(target_arch = "wasm32"))]- std::thread::spawn(job);+ std::thread::Builder::new().name("lc-import-scan".into()).spawn(job).map_err(|e| format!("could not start: {e}"))?; #[cfg(target_arch = "wasm32")] job(); let guard = scan_guard(app, &progress, false);@@ -134,7 +134,7 @@ pub fn browse(app: &mut LightcraftApp, path: &str, subfolders: Option<bool>) -> let _ = tx.send(scan_with(input, &files, &p)); }; #[cfg(not(target_arch = "wasm32"))]- std::thread::spawn(job);+ std::thread::Builder::new().name("lc-browse-scan".into()).spawn(job).map_err(|e| format!("could not start: {e}"))?; #[cfg(target_arch = "wasm32")] job(); let guard = scan_guard(app, &progress, true);@@ -409,8 +409,12 @@ pub(super) fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize running.store(false, Relaxed); ctx.request_repaint(); };+ // (a thread that can't start keeps the last count; the count is tried again once it is stale) #[cfg(not(target_arch = "wasm32"))]- std::thread::spawn(work);+ if let Err(e) = std::thread::Builder::new().name("lc-missing".into()).spawn(work) {+ log::warn!("counting missing photos: could not start: {e}");+ job.running.store(false, Relaxed);+ } #[cfg(target_arch = "wasm32")] work(); ui.data_mut(|d| d.insert_temp(id, job.clone()));@@ -55,7 +55,7 @@ pub fn open(app: &mut LightcraftApp, path: &str, name: &str, disk: bool) -> Resu let _ = tx.send(scan_with(input, &p)); }; #[cfg(not(target_arch = "wasm32"))]- std::thread::spawn(job);+ std::thread::Builder::new().name("lc-sync-scan".into()).spawn(job).map_err(|e| format!("could not start: {e}"))?; #[cfg(target_arch = "wasm32")] job(); let guard = app.session.activity.start("sync", "Looking for changes", Cancel::Flag(progress.files.cancel.clone()));@@ -270,7 +270,7 @@ pub fn confirm(app: &mut LightcraftApp, dlg: &Dialog) -> Result<Value, String> { } }; #[cfg(not(target_arch = "wasm32"))]- std::thread::spawn(job);+ std::thread::Builder::new().name("lc-sync".into()).spawn(job).map_err(|e| format!("could not start: {e}"))?; #[cfg(target_arch = "wasm32")] job(); let guard = app.session.activity.start("sync", "Synchronizing folder", Cancel::Flag(progress.files.cancel.clone()));