From 8cb7d34cea7ba776acce1e8a24ce2c83f6827cd4 Mon Sep 17 00:00:00 2001
From: Benjamin Woodruff <github@benjam.info>
Date: Sun, 26 Feb 2023 12:25:51 -0800
Subject: [PATCH] core: Mark DefaultGuard as !Send (#2482)

The `Drop` impl of `DefaultGuard` modifies the `CURRENT_STATE` thread
local, assuming it to be the same thread local it was constructed with.

However, because `DefaultGuard` is `Send`, that may be a different
thread local.

Therefore, we should mark `DefaultGuard` as `!Send`.
---
 tracing-core/src/dispatch.rs | 37 +++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/tracing-core/src/dispatch.rs b/tracing-core/src/dispatch.rs
index 555b3a1f45..581c883f1a 100644
--- a/tracing-core/src/dispatch.rs
+++ b/tracing-core/src/dispatch.rs
@@ -140,7 +140,7 @@ use crate::{
     span, Event, LevelFilter, Metadata,
 };
 
-use core::{any::Any, fmt, sync::atomic::Ordering};
+use core::{any::Any, fmt, marker::PhantomData, sync::atomic::Ordering};
 
 #[cfg(feature = "portable-atomic")]
 use portable_atomic::{AtomicBool, AtomicUsize};
@@ -263,12 +263,26 @@ struct State {
 #[cfg(feature = "std")]
 struct Entered<'a>(&'a State);
 
+/// A trait that implements `Sync`, but not `Send`. Used with PhantomData, this
+/// can mark a struct as `!Send`.
+#[cfg(feature = "std")]
+trait NotSend: Sync {}
+
 /// A guard that resets the current default dispatcher to the prior
 /// default dispatcher when dropped.
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
 #[derive(Debug)]
-pub struct DefaultGuard(Option<Dispatch>);
+pub struct DefaultGuard(
+    Option<Dispatch>,
+    /// ```compile_fail
+    /// use tracing_core::dispatch::*;
+    /// trait AssertSend: Send {}
+    ///
+    /// impl AssertSend for DefaultGuard {}
+    /// ```
+    PhantomData<dyn NotSend>,
+);
 
 /// Sets this dispatch as the default for the duration of a closure.
 ///
@@ -297,8 +311,8 @@ pub fn with_default<T>(dispatcher: &Dispatch, f: impl FnOnce() -> T) -> T {
     f()
 }
 
-/// Sets the dispatch as the default dispatch for the duration of the lifetime
-/// of the returned DefaultGuard
+/// Sets this dispatch as the current thread's default dispatch for the duration
+/// of the lifetime of the returned [`DefaultGuard`].
 ///
 /// <div class="example-wrap" style="display:inline-block">
 /// <pre class="ignore" style="white-space:normal;font:inherit;">
@@ -307,6 +321,16 @@ pub fn with_default<T>(dispatcher: &Dispatch, f: impl FnOnce() -> T) -> T {
 /// `no_std` users should use [`set_global_default`] instead.
 ///
 /// </pre></div>
+///
+/// <div class="example-wrap" style="display:inline-block">
+/// <pre class="ignore" style="white-space:normal;font:inherit;">
+///
+/// **Note**: Because this sets the dispatch for the current thread only, the returned
+/// [`DefaultGuard`] does not implement [`Send`]. If the guard was sent to another thread and
+/// dropped there, we'd try to restore the dispatch value for the wrong thread. Thus,
+/// [`DefaultGuard`] should not be sent between threads.
+///
+/// </pre></div>
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
 #[must_use = "Dropping the guard unregisters the dispatcher."]
@@ -1030,7 +1054,7 @@ impl State {
             .ok();
         EXISTS.store(true, Ordering::Release);
         SCOPED_COUNT.fetch_add(1, Ordering::Release);
-        DefaultGuard(prior)
+        DefaultGuard(prior, PhantomData)
     }
 
     #[inline]
@@ -1093,6 +1117,9 @@ mod test {
         metadata::{Kind, Level, Metadata},
     };
 
+    trait AssertSync: Sync {}
+    impl AssertSync for DefaultGuard {}
+
     #[test]
     fn dispatch_is() {
         let dispatcher = Dispatch::from_static(&NO_COLLECTOR);