From 2cfb160f784809d6d5d29b10f240c57799c49e78 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 7 Dec 2023 04:17:39 +0100 Subject: [PATCH] Add Main Thread Checker --- README.md | 8 ++- src/main.rs | 77 ++++++++++++++++++++--- test/ci.sh | 17 +++++ test/test-main_thread_checker/Cargo.lock | 32 ++++++++++ test/test-main_thread_checker/Cargo.toml | 8 +++ test/test-main_thread_checker/src/main.rs | 18 ++++++ 6 files changed, 152 insertions(+), 8 deletions(-) create mode 100644 test/test-main_thread_checker/Cargo.lock create mode 100644 test/test-main_thread_checker/Cargo.toml create mode 100644 test/test-main_thread_checker/src/main.rs diff --git a/README.md b/README.md index 3440eae..c29b623 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ itself. ### Sanitizing `cargo careful` can additionally build and run your program and standard library -with a sanitizer. This feature is experimental and disabled by default. +with a sanitizer. This feature is experimental and disabled by default. The [underlying `rustc` feature](https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/sanitizer.html) doesn't play well with [procedural macros](https://doc.rust-lang.org/reference/procedural-macros.html). @@ -86,6 +86,12 @@ setting `ASAN_OPTIONS=detect_leaks=0` in your program's environment, as memory l usually a soundness or correctness issue. If you set the `ASAN_OPTIONS` environment variable yourself (to any value, including an empty string), that will override this behavior. +### Main Thread Checker + +`cargo careful` automatically enables [Apple's Main Thread Checker](https://developer.apple.com/documentation/xcode/diagnosing-memory-thread-and-crash-issues-early#Detect-improper-UI-updates-on-background-threads) on macOS, iOS, tvOS and watchOS targets, whenever the user has Xcode installed. + +This helps diagnosing issues with executing thread-unsafe functionality off the main thread on those platforms. + ### `cfg` flag `cargo careful` sets the `careful` configuration flag, so you can use Rust's compile-time diff --git a/src/main.rs b/src/main.rs index 399705b..85e008d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,9 +3,9 @@ use std::env; use std::ffi::OsString; use std::path::PathBuf; -use std::process::{self, Command}; +use std::process::{self, Command, Stdio}; -use anyhow::{bail, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use rustc_build_sysroot::{BuildMode, SysrootBuilder, SysrootConfig}; use rustc_version::VersionMeta; @@ -38,6 +38,48 @@ pub fn rustc_version_info() -> VersionMeta { VersionMeta::for_command(rustc()).expect("failed to determine rustc version") } +/// Find the path for Apple's Main Thread Checker on the current system. +/// +/// This is intended to be used on macOS, but should work on other systems +/// that has something similar to XCode set up. +fn main_thread_checker_path() -> Result> { + // Find the Xcode developer directory, usually one of: + // - /Applications/Xcode.app/Contents/Developer + // - /Library/Developer/CommandLineTools + // + // This could be done by the `apple-sdk` crate, but we avoid the dependency here. + let output = Command::new("xcode-select") + .args(["--print-path"]) + .stderr(Stdio::null()) + .output() + .context("`xcode-select --print-path` failed to run")?; + + if !output.status.success() { + return Err(anyhow!( + "got error when running `xcode-select --print-path`: {}", + output.status, + )); + } + + let stdout = String::from_utf8(output.stdout) + .context("`xcode-select --print-path` returned invalid UTF-8")?; + let developer_dir = PathBuf::from(stdout.trim()); + + // Introduced in XCode 9.0, and has not changed location since. + // + let path = developer_dir.join("usr/lib/libMainThreadChecker.dylib"); + if path.try_exists()? { + Ok(Some(path)) + } else { + eprintln!( + "warn: libMainThreadChecker.dylib could not be found at {}", + path.display() + ); + eprintln!(" This usually means you're using the Xcode command line tools, which does not have this capability."); + Ok(None) + } +} + // Computes the extra flags that need to be passed to cargo to make it behave like the current // cargo invocation. fn cargo_extra_flags() -> Vec { @@ -235,7 +277,7 @@ fn build_sysroot( sysroot_dir } -fn cargo_careful(args: env::Args) { +fn cargo_careful(args: env::Args) -> Result<()> { let mut args = args.peekable(); let rustc_version = rustc_version_info(); @@ -285,6 +327,7 @@ fn cargo_careful(args: env::Args) { // Forward regular argument. cargo_args.push(arg); } + // The rest is for cargo to forward to the binary / test runner. cargo_args.push("--".into()); cargo_args.extend(args); @@ -317,7 +360,7 @@ fn cargo_careful(args: env::Args) { Some(c) => c, None => { // We just did the setup. - return; + return Ok(()); } }; @@ -339,6 +382,26 @@ fn cargo_careful(args: env::Args) { cmd.args(["--target", target.as_str()]); } + // Enable Main Thread Checker on Apple platforms, as documented here: + // + let apple_target = target.contains("-darwin") + || target.contains("-ios") + || target.contains("-tvos") + || target.contains("-watchos"); + // FIXME: We only do this if the host is running on macOS, since cargo + // doesn't currently have a good way of passing environment variables only + // to target binaries when using `cargo run` or `cargo test` (and this + // means that `rustc` is also run under the main thread checker, which will + // probably fail on other platforms). + // See . + if apple_target && cfg!(target_os = "macos") { + if let Some(path) = main_thread_checker_path()? { + cmd.arg("--config"); + // TODO: Quote the path correctly according to toml rules + cmd.arg(format!("env.DYLD_INSERT_LIBRARIES={path:?}")); + } + } + cmd.args(cargo_args); // Setup environment. Both rustc and rustdoc need these flags. @@ -357,10 +420,10 @@ fn cargo_careful(args: env::Args) { } // Run it! - exec(cmd, (verbose > 0).then_some("[cargo-careful] ")); + exec(cmd, (verbose > 0).then_some("[cargo-careful] ")) } -fn main() { +fn main() -> Result<()> { let mut args = env::args(); // Skip binary name. args.next().unwrap(); @@ -373,7 +436,7 @@ fn main() { match first.as_str() { "careful" => { // It's us! - cargo_careful(args); + cargo_careful(args) } _ => { show_error!( diff --git a/test/ci.sh b/test/ci.sh index 2490e22..cd0d67b 100755 --- a/test/ci.sh +++ b/test/ci.sh @@ -18,3 +18,20 @@ cargo careful setup --target x86_64-unknown-none cargo careful build --target x86_64-unknown-none --locked cargo clean popd + +# test Apple's Main Thread Checker +if uname -s | grep -q "Darwin" +then + pushd test-main_thread_checker + # Run as normal; this will output warnings, but not fail + cargo careful run --locked + # Run with flag that tells the Main Thread Checker to fail + # See + if MTC_CRASH_ON_REPORT=1 cargo careful run --locked + then + echo "Main Thread Checker did not crash" + exit 1 + fi + cargo clean + popd +fi diff --git a/test/test-main_thread_checker/Cargo.lock b/test/test-main_thread_checker/Cargo.lock new file mode 100644 index 0000000..17b3392 --- /dev/null +++ b/test/test-main_thread_checker/Cargo.lock @@ -0,0 +1,32 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "objc-sys" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7c71324e4180d0899963fc83d9d241ac39e699609fc1025a850aadac8257459" + +[[package]] +name = "objc2" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a9c7f0d511a4ce26b078183179dca908171cfc69f88986fe36c5138e1834476" +dependencies = [ + "objc-sys", + "objc2-encode", +] + +[[package]] +name = "objc2-encode" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ff06a6505cde0766484f38d8479ac8e6d31c66fbc2d5492f65ca8c091456379" + +[[package]] +name = "test-cargo-careful-main_thread_checker" +version = "0.1.0" +dependencies = [ + "objc2", +] diff --git a/test/test-main_thread_checker/Cargo.toml b/test/test-main_thread_checker/Cargo.toml new file mode 100644 index 0000000..d7c88d9 --- /dev/null +++ b/test/test-main_thread_checker/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "test-cargo-careful-main_thread_checker" +version = "0.1.0" +edition = "2021" +publish = false + +[target.'cfg(target_os = "macos")'.dependencies] +objc2 = "0.5.0" diff --git a/test/test-main_thread_checker/src/main.rs b/test/test-main_thread_checker/src/main.rs new file mode 100644 index 0000000..b9922a6 --- /dev/null +++ b/test/test-main_thread_checker/src/main.rs @@ -0,0 +1,18 @@ +//! Run [NSView new] on a separate thread, which should get caught by the +//! main thread checker. +use objc2::rc::Id; +use objc2::runtime::AnyObject; +use objc2::{class, msg_send_id}; + +#[link(name = "AppKit", kind = "framework")] +extern "C" {} + +fn main() { + std::thread::scope(|s| { + s.spawn(|| { + // Note: Usually you'd use `icrate::NSView::new`, this is to + // avoid the heavy dependency. + let _: Id = unsafe { msg_send_id![class!(NSView), new] }; + }); + }); +}