diff --git a/Cargo.lock b/Cargo.lock index f34d2c1c65bb..58d56512b35f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4128,6 +4128,7 @@ dependencies = [ "ndarray-rand", "nohash-hasher", "once_cell", + "parking_lot 0.12.1", "rand", "re_build_build_info", "re_build_info", diff --git a/Cranky.toml b/Cranky.toml index aab5196bce36..c6ed9f2f54c6 100644 --- a/Cranky.toml +++ b/Cranky.toml @@ -1,5 +1,6 @@ # https://github.com/ericseppanen/cargo-cranky # cargo install cargo-cranky && cargo cranky +# See also clippy.toml deny = [ "unsafe_code", @@ -17,8 +18,11 @@ warn = [ "clippy::dbg_macro", "clippy::debug_assert_with_mut_call", "clippy::derive_partial_eq_without_eq", - "clippy::disallowed_methods", - "clippy::disallowed_script_idents", + "clippy::disallowed_macros", # See clippy.toml + "clippy::disallowed_methods", # See clippy.toml + "clippy::disallowed_names", # See clippy.toml + "clippy::disallowed_script_idents", # See clippy.toml + "clippy::disallowed_types", # See clippy.toml "clippy::doc_link_with_quotes", "clippy::doc_markdown", "clippy::empty_enum", diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000000..cbeea80f79e8 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,56 @@ +msrv = "1.67" + + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros +disallowed-macros = [ + 'dbg', + + # TODO(emilk): consider forbidding these to encourage the use of proper log stream, and then explicitly allow legitimate uses + # 'std::eprint', + # 'std::eprintln', + # 'std::print', + # 'std::println', + + # 'std::unimplemented', # generated by ArrowDeserialize derive-macro :( +] + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods +disallowed-methods = [ + "std::env::temp_dir", # Use the tempdir crate instead + + # Disabled because of https://github.com/rust-lang/rust-clippy/issues/10406 + # "std::time::Instant::now", # use `instant` crate instead for wasm/web compatability + "std::time::Duration::elapsed", # use `instant` crate instead for wasm/web compatability + "std::time::SystemTime::now", # use `instant` or `time` crates instead for wasm/web compatability + + "std::thread::spawn", # Use `std::thread::Builder` and name the thread + + "sha1::Digest::new", # SHA1 is cryptographically broken +] + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names +disallowed-names = [] + +# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types +disallowed-types = [ + # Use the faster & simpler non-poisonable primitives in `parking_lot` instead + "std::sync::Mutex", + "std::sync::RwLock", + "std::sync::Condvar", + # "std::sync::Once", # enabled for now as the `log_once` macro uses it internally + + "ring::digest::SHA1_FOR_LEGACY_USE_ONLY", # SHA1 is cryptographically broken +] + +# Allow-list of words for markdown in dosctrings https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown +doc-valid-idents = [ + "GLB", + "GLTF", + "iOS", + "macOS", + "NaN", + "OBJ", + "sRGB", + "sRGBA", + "WebGL", +] diff --git a/crates/re_analytics/src/pipeline_native.rs b/crates/re_analytics/src/pipeline_native.rs index 3b2920c9655b..68262bac3c4f 100644 --- a/crates/re_analytics/src/pipeline_native.rs +++ b/crates/re_analytics/src/pipeline_native.rs @@ -75,7 +75,7 @@ impl Pipeline { // The eventual part comes from the fact that this only runs as part of the Rerun viewer, // and as such there's no guarantee it will ever run again, even if there's pending data. - _ = std::thread::Builder::new() + if let Err(err) = std::thread::Builder::new() .name("pipeline_catchup".into()) .spawn({ let config = config.clone(); @@ -88,9 +88,12 @@ impl Pipeline { let res = flush_pending_events(&config, &sink); trace!(%analytics_id, %session_id, ?res, "pipeline catchup thread shut down"); } - }); + }) + { + re_log::warn!("Failed to spawn analytics thread: {err}"); + } - _ = std::thread::Builder::new().name("pipeline".into()).spawn({ + if let Err(err) = std::thread::Builder::new().name("pipeline".into()).spawn({ let config = config.clone(); let event_tx = event_tx.clone(); move || { @@ -102,7 +105,9 @@ impl Pipeline { realtime_pipeline(&config, &sink, session_file, tick, &event_tx, &event_rx); trace!(%analytics_id, %session_id, ?res, "pipeline thread shut down"); } - }); + }) { + re_log::warn!("Failed to spawn analytics thread: {err}"); + } Ok(Some(Self { event_tx })) } @@ -269,7 +274,7 @@ fn append_event( // corrupt row in the analytics file, that we'll simply discard later on. // We'll try to write a linefeed one more time, just in case, to avoid potentially // impacting other events. - _ = session_file.write_all(b"\n"); + session_file.write_all(b"\n").ok(); warn!(%err, %analytics_id, %session_id, "couldn't write to analytics data file"); return Err(event); } diff --git a/crates/re_log_types/src/component_types/mesh3d.rs b/crates/re_log_types/src/component_types/mesh3d.rs index d410827b1f0c..d12567b17655 100644 --- a/crates/re_log_types/src/component_types/mesh3d.rs +++ b/crates/re_log_types/src/component_types/mesh3d.rs @@ -326,7 +326,7 @@ impl ArrowDeserialize for EncodedMesh3D { // ---------------------------------------------------------------------------- -/// The format of a binary mesh file, e.g. `GLTF`, `GLB`, `OBJ` +/// The format of a binary mesh file, e.g. GLTF, GLB, OBJ #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, ArrowField, ArrowSerialize, ArrowDeserialize)] #[arrow_field(type = "dense")] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] diff --git a/crates/re_sdk/Cargo.toml b/crates/re_sdk/Cargo.toml index becd88f5ac36..7cfecd282730 100644 --- a/crates/re_sdk/Cargo.toml +++ b/crates/re_sdk/Cargo.toml @@ -54,7 +54,6 @@ re_sdk_comms = { workspace = true, features = ["client"] } re_smart_channel.workspace = true re_string_interner.workspace = true - anyhow.workspace = true arrow2.workspace = true crossbeam = "0.8" @@ -62,6 +61,7 @@ document-features = "0.2" lazy_static.workspace = true nohash-hasher = "0.2" once_cell = "1.12" +parking_lot = "0.12" thiserror.workspace = true # Optional dependencies: diff --git a/crates/re_sdk/src/global.rs b/crates/re_sdk/src/global.rs index cee01c97031a..0a3967e0431f 100644 --- a/crates/re_sdk/src/global.rs +++ b/crates/re_sdk/src/global.rs @@ -4,7 +4,7 @@ use crate::session::Session; /// /// By default, logging is enabled. To disable logging, call `set_enabled(false)` on the global `Session`, or /// set the `RERUN` environment variable to `false`. -pub fn global_session() -> std::sync::MutexGuard<'static, Session> { +pub fn global_session() -> parking_lot::MutexGuard<'static, Session> { let default_enabled = true; global_session_with_default_enabled(default_enabled) } @@ -15,11 +15,11 @@ pub fn global_session() -> std::sync::MutexGuard<'static, Session> { /// It can be overridden with the `RERUN` environment variable. pub fn global_session_with_default_enabled( default_enabled: bool, -) -> std::sync::MutexGuard<'static, Session> { +) -> parking_lot::MutexGuard<'static, Session> { use once_cell::sync::OnceCell; - use std::sync::Mutex; + use parking_lot::Mutex; static INSTANCE: OnceCell> = OnceCell::new(); let mutex = INSTANCE.get_or_init(|| Mutex::new(Session::with_default_enabled(default_enabled))); - mutex.lock().unwrap() + mutex.lock() } diff --git a/crates/re_sdk/src/session.rs b/crates/re_sdk/src/session.rs index 717867d983f6..d96940348842 100644 --- a/crates/re_sdk/src/session.rs +++ b/crates/re_sdk/src/session.rs @@ -462,7 +462,10 @@ impl Session { let app_env = self.app_env(); // NOTE: Forget the handle on purpose, leave that thread be. - _ = std::thread::spawn(move || run(self)); + std::thread::Builder::new() + .name("spawned".into()) + .spawn(move || run(self)) + .expect("Failed to spawn thread"); // NOTE: Some platforms still mandate that the UI must run on the main thread, so make sure // to spawn the viewer in place and migrate the user callback to a new thread. diff --git a/run_wasm/src/main.rs b/run_wasm/src/main.rs index 4fd4dabbb012..b682b0bdefd0 100644 --- a/run_wasm/src/main.rs +++ b/run_wasm/src/main.rs @@ -99,9 +99,12 @@ fn main() { let host = host.as_deref().unwrap_or("localhost"); let port = port.as_deref().unwrap_or("8000"); - std::thread::spawn(|| { - cargo_run_wasm::run_wasm_with_css(CSS); - }); + std::thread::Builder::new() + .name("cargo_run_wasm".into()) + .spawn(|| { + cargo_run_wasm::run_wasm_with_css(CSS); + }) + .expect("Failed to spawn thread"); // Wait for the server to be up before opening a browser tab. let addr = format!("{host}:{port}")