Skip to content

Commit 6236e70

Browse files
committed
core: Mark DefaultGuard as !Send (tokio-rs#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`.
1 parent a0126b2 commit 6236e70

File tree

1 file changed

+32
-4
lines changed

1 file changed

+32
-4
lines changed

tracing-core/src/dispatch.rs

+32-4
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ use crate::{
143143
use core::{
144144
any::Any,
145145
fmt,
146+
marker::PhantomData,
146147
sync::atomic::{AtomicBool, AtomicUsize, Ordering},
147148
};
148149

@@ -256,12 +257,27 @@ struct State {
256257
#[cfg(feature = "std")]
257258
struct Entered<'a>(&'a State);
258259

260+
/// A trait that implements `Sync`, but not `Send`. Used with PhantomData, this
261+
/// can mark a struct as `!Send`.
262+
#[cfg(feature = "std")]
263+
trait NotSend: Sync {}
264+
259265
/// A guard that resets the current default dispatcher to the prior
260266
/// default dispatcher when dropped.
261267
#[cfg(feature = "std")]
262268
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
263269
#[derive(Debug)]
264-
pub struct DefaultGuard(Option<Dispatch>);
270+
pub struct DefaultGuard(
271+
Option<Dispatch>,
272+
273+
/// ```compile_fail
274+
/// use tracing_core::dispatch::*;
275+
/// trait AssertSend: Send {}
276+
///
277+
/// impl AssertSend for DefaultGuard {}
278+
/// ```
279+
PhantomData<dyn NotSend>,
280+
);
265281

266282
/// Sets this dispatch as the default for the duration of a closure.
267283
///
@@ -290,15 +306,24 @@ pub fn with_default<T>(dispatcher: &Dispatch, f: impl FnOnce() -> T) -> T {
290306
f()
291307
}
292308

293-
/// Sets the dispatch as the default dispatch for the duration of the lifetime
294-
/// of the returned DefaultGuard
309+
/// Sets the dispatch as the current thread's default dispatch for the duration
310+
/// of the lifetime of the returned DefaultGuard.
295311
///
296312
/// <div class="example-wrap" style="display:inline-block">
297313
/// <pre class="ignore" style="white-space:normal;font:inherit;">
298314
///
299315
/// **Note**: This function required the Rust standard library.
300316
/// `no_std` users should use [`set_global_default`] instead.
317+
/// </pre></div>
301318
///
319+
/// <div class="example-wrap" style="display:inline-block">
320+
/// <pre class="ignore" style="white-space:normal;font:inherit;">
321+
///
322+
/// **Note**: Because this sets the dispatch for the current thread only, the
323+
/// returned [`DefaultGuard`] does not implement [`Send`]. If the guard was sent
324+
/// to another thread and dropped there, we'd try to restore the dispatch value
325+
/// for the wrong thread. Thus, [`DefaultGuard`] should not be sent between
326+
/// threads.
302327
/// </pre></div>
303328
#[cfg(feature = "std")]
304329
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
@@ -1009,7 +1034,7 @@ impl State {
10091034
.ok();
10101035
EXISTS.store(true, Ordering::Release);
10111036
SCOPED_COUNT.fetch_add(1, Ordering::Release);
1012-
DefaultGuard(prior)
1037+
DefaultGuard(prior, PhantomData)
10131038
}
10141039

10151040
#[inline]
@@ -1072,6 +1097,9 @@ mod test {
10721097
metadata::{Kind, Level, Metadata},
10731098
};
10741099

1100+
trait AssertSync: Sync {}
1101+
impl AssertSync for DefaultGuard {}
1102+
10751103
#[test]
10761104
fn dispatch_is() {
10771105
let dispatcher = Dispatch::from_static(&NO_COLLECTOR);

0 commit comments

Comments
 (0)