Merge pull request #24 from madsmtm/apple-main-thread-checker

Add Main Thread Checker

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

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 have something similar to XCode set up.
+fn main_thread_checker_path() -> Result<Option<PathBuf>> {
+    // 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`:\n{:?}",
+            output,
+        ));
+    }
+
+    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.
+    // <https://developer.apple.com/library/archive/releasenotes/DeveloperTools/RN-Xcode/Chapters/Introduction.html#//apple_ref/doc/uid/TP40001051-CH1-SW974>
+    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<String> {
@@ -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,34 @@ fn cargo_careful(args: env::Args) {
         cmd.args(["--target", target.as_str()]);
     }
 
+    // Enable Main Thread Checker on macOS targets, as documented here:
+    // <https://developer.apple.com/documentation/xcode/diagnosing-memory-thread-and-crash-issues-early#Detect-improper-UI-updates-on-background-threads>
+    //
+    // On iOS, tvOS and watchOS simulators, the path is somewhere inside the
+    // simulator runtime, which is more difficult to find, so we don't do that
+    // yet (those target also probably wouldn't run in `cargo-careful` anyway).
+    //
+    // Note: The main thread checker by default removes itself from
+    // `DYLD_INSERT_LIBRARIES` upon load, see `MTC_RESET_INSERT_LIBRARIES`:
+    // <https://bryce.co/main-thread-checker-configuration/#mtc_reset_insert_libraries>
+    // This means that it is not inherited by child processes, so we have to
+    // tell Cargo to set this environment variable for the processes it
+    // launches (instead of just setting it for Cargo itself using `cmd.env`).
+    //
+    // Note: We do this even if the host is not running macOS, even though the
+    // environment variable will also be passed to any rustc processes that
+    // Cargo spawns (as Cargo doesn't currently have a good way of only
+    // specifying environment variables to only the binary being run).
+    // This is probably fine though, the environment variable is
+    // Apple-specific and will likely be ignored on other hosts.
+    if target.contains("-darwin") {
+        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 +428,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 +444,7 @@ fn main() {
     match first.as_str() {
         "careful" => {
             // It's us!
-            cargo_careful(args);
+            cargo_careful(args)
         }
         _ => {
             show_error!(

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 <https://bryce.co/main-thread-checker-configuration/>
+    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

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"

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<AnyObject> = unsafe { msg_send_id![class!(NSView), new] };
+        });
+    });
+}