fix(settings): mark drop guard as !Send (#695)

Fixes #694

Because the `SettingsDropGuard` modifies a `ThreadLocalKey` (it clones
it in the `Settings::bind_to_scope` method, then restores it in its drop
impl), it should not be moved to a different thread, as then it would
modify that thread's `CURRENT_SETTINGS` instead of the original
thread's.

This is not an issue with normal code as the guard is usually unnameable
(`let _guard = settings.bind_to_scope();`). but it is a problem with
`async` code, as when held across `await` points is can be moved by the
runtime to a separate thread.

We fix it by adding a `PhantomData` field to `SettingsDropGuard` that
does not implement `Send`. We verify it works by adding a `compile_fail`
doctest to the struct that tries to send the drop guard to a different
thread.

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 All notable changes to insta and cargo-insta are documented here.
 
+## Unreleased
+
+- `insta::internals::SettingsBindDropGuard` (returned from
+  `Settings::bind_to_scope`) no longer implements `Send`. This was an error and
+  any tests relying on this behavior where not working properly.
+  Fixes #694 in #695 by @jalil-salame
+
 ## 1.41.1
 
 - Re-release of 1.41.1 to generate release artifacts correctly.

insta/src/settings.rs

@@ -569,7 +569,7 @@ impl Settings {
         CURRENT_SETTINGS.with(|x| {
             let mut x = x.borrow_mut();
             let old = mem::replace(&mut x.inner, self.inner.clone());
-            SettingsBindDropGuard(Some(old))
+            SettingsBindDropGuard(Some(old), std::marker::PhantomData)
         })
     }
 
@@ -580,8 +580,29 @@ impl Settings {
 }
 
 /// Returned from [`Settings::bind_to_scope`]
+///
+/// This type is not shareable between threads:
+///
+/// ```compile_fail E0277
+/// let mut settings = insta::Settings::clone_current();
+/// settings.set_snapshot_suffix("test drop guard");
+/// let guard = settings.bind_to_scope();
+///
+/// std::thread::spawn(move || { let guard = guard; }); // doesn't compile
+/// ```
+///
+/// This is to ensure tests under async runtimes like `tokio` don't show unexpected results
 #[must_use = "The guard is immediately dropped so binding has no effect. Use `let _guard = ...` to bind it."]
-pub struct SettingsBindDropGuard(Option<Arc<ActualSettings>>);
+pub struct SettingsBindDropGuard(
+    Option<Arc<ActualSettings>>,
+    /// A ZST that is not [`Send`] but is [`Sync`]
+    ///
+    /// This is necessary due to the lack of stable [negative impls](https://github.com/rust-lang/rust/issues/68318).
+    ///
+    /// Required as [`SettingsBindDropGuard`] modifies a thread local variable which would end up
+    /// with unexpected results if sent to a different thread.
+    std::marker::PhantomData<std::sync::MutexGuard<'static, ()>>,
+);
 
 impl Drop for SettingsBindDropGuard {
     fn drop(&mut self) {