From a8cb54c07077da0e964ac42ac2718bb6c0aaad31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?=
 <jalil.salame@gmail.com>
Date: Sat, 14 Dec 2024 12:56:42 +0100
Subject: [PATCH 1/2] fix(settings): mark drop guard as !Send

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.
---
 insta/src/settings.rs | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/insta/src/settings.rs b/insta/src/settings.rs
index 1a1f4425..716f8ae1 100644
--- a/insta/src/settings.rs
+++ b/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) {

From 2b3626c029af56ed4389977a948397acc839a32d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?=
 <jalil.salame@gmail.com>
Date: Mon, 16 Dec 2024 20:11:05 +0100
Subject: [PATCH 2/2] chore: update changelog to reflect changes in #695

---
 CHANGELOG.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6c5f8f1c..8786cb2a 100644
--- a/CHANGELOG.md
+++ b/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.